Things are compiled under ./env, but ./env / ./env.in is for running examples/tests/... only

[emixa-d]

(This is response to bug report at guile-user ML, not yet in archives)

Running

./pre-inst-env guix build --target=i586-pc-gnu --system=i686-linux guile-fibers
make[1]: Entering directory '/tmp/guix-build-guile-fibers-1.3.1.drv-0/source'
./env
/gnu/store/6f0nwq8wfcrbii9lxlffjph7kajgp6q9-guile-3.0.9/bin/guild compile --target=i586-pc-gnu -L "/tmp/guix-build-guile-fibers-1.3.1.drv-0/source"
-Wunbound-variable -Warity-mismatch -Wformat -o "fibers/operations.go" "fibers/operations.scm"
[…]
In unknown file:
5 (primitive-load-path "fibers/events-impl" #<procedure 29b290 at ice-9/boot-9.scm…>)

Looking at the backtrace, while compiling (fibers operations), it loads (fibers events-impl).

In override/fibers/events-impl.scm:
41:15 4 (_)

In particular, it loads the override. The override/non-override was added in 62b2ca6 to make sure that when compiling, no .so are loaded! The override contains the .so loading code, the non-override fibers/events-impl.scm is a stub that’s completely safe for (cross-)compilation.

So, it has no business loading override/fibers/events-impl.scm! The only place where ‘override’ is added to the load path is in ‘env.in/env’, which is only used for testing (not for actual compilation).

Yet, the make says ‘./env [guild compile stuff’ – that ‘./env’ shouldn’t be appearing there (it’s for running tests / examples (runtime things), not compilation). Looking at https://github.com/wingo/fibers/actions/runs/11133129373/job/30938526065, this ‘./env’ also mysteriously appears upstream.

So, what adds ./env and how do we stop it? (I don’t have an appropriate system for investigation at the moment.)

[emixa-d]

As mentioned by @janneke, here ./env comes in:

https://github.com/wingo/fibers/blob/f92e5cb4f78e7e3d3537bfc9622bc59ea99fe9a7/build-aux/guile.am#L26C1

[emixa-d]

@civodul, could you give removing './env' a try (I don't have a good set up at the moment)?

[emixa-d]

On a maybe related matter, I got the impression that

fibers/fibers/config.scm.in

Line 23 in 01f475f

(cond

isn't completely correct, or became redundant. If

fibers/fibers/config.scm.in

Line 23 in 01f475f

(cond

did its job correctly, then extension-library would never be run during compilation (excluding tests). So, replacing it by an unconditional @extlibdir@ should work.

[emixa-d]

My preference would be to go back to the FIBERS_CROSS_COMPILING environment variable trick that was used prior to 62b2ca6 (I use the same trick in most projects), but I'm open to other solutions.

I would rather avoid environment variable pollution.

This change was proposed in #77. It seems to me that a simpler solution to the "problem" mentioned in events-impl.scm would have been to set FIBERS_CROSS_COMPILING in Makefile.am instead of setting it in ./env, for instance.

But then it would still do an environment variable lookup, and then users of Fibers have one more thing to worry about. Eliminating the environment variable eliminates risks.

For example, what if an implementation of 'sh' written in Guile and using Fibers is used?

Thoughts?

See removing ./env from the invocation, as I proposed to janneke. It doesn't seem to serve any purpose anymore.

[civodul]

@civodul, could you give removing './env' a try (I don't have a good set up at the moment)?

As I wrote in #114, Guix carries a patch by @janneke that removes ./env but only when cross-compiling. ./env is still needed when not cross-compiling so one can run tests.

So I'm not a fan of this approach. My preference goes to something like the FIBERS_CROSS_COMPILING environment variable, as suggested in #114. One thing's not clear to me: how did #77 improve on that?

[civodul]

Oh, our messages crossed. I agree that it's best if we can avoid that environment variable lookup at run time, just not at any cost.

Anyway, I have limited bandwidth so any fix is good to take!

[civodul]

Also, maybe we can remove the whole *-impl.scm + override/ machinery if it doesn't do the job it was intended for?

[emixa-d]

It's needed for tests, that's what TESTS_ENVIRONMENT is for. IIUC, it only needs to be removed in guile.am, you don't need to remove ./env itself.

how did #77 improve on that?

It eliminates the FIBERS_CROSS_COMPILING variable. No extra environment variable, no problems with anyone else setting this variable and causing trouble, also if we don't tweak that variable, then nobody else can be bothered by it.

Also, maybe we can remove the whole *-impl.scm + override/ machinery if it doesn't do the job it was intended for?

Maybe try removing './env' from guile.m4 first?

[emixa-d]

I think the ideal solution would be to modify Guile, such that you can tell it that 'module X needs module Y, but only for procedures/variables/..., not for macros'. Then no override mechanism nor FIBERS_CROSS_COMPILING would be needed. Then, when compiling X, only X is compiled (without loading Y(*)).

(*) exception: would be nice if it could still look at the .go for defined/undefinedness, types, inlining, but without actually properly loading it, or something

Perhaps eval-when can already accomplish this, but I don't know if use-modules works fine under eval-when and whether the compiler can deal with.

(That's also insufficient for portable RnRS -- perhaps 'phases' is an option, but I'm not familiar enough with them to be sure, and Guile always does the implicit phasing thing.)

[civodul]

It's needed for tests, that's what TESTS_ENVIRONMENT is for. IIUC, it only needs to be removed in guile.am, you don't need to remove ./env itself.

The *-impl.scm + override/ machinery didn't exist prior to #77; it is not "needed for tests". My understanding is that it was introduced to support cross-compilation without having that extra FIBERS_CROSS_COMPILING environment variable---a laudable goal, but it failed to provide both.

Maybe try removing './env' from guile.m4 first?

./env sets up the .scm and .go and .so search paths, so I think it's good to use it when running guild compile; otherwise we'd have to basically duplicate these environment variable settings in the .scm.go target. Or am I misunderstanding something?

[emixa-d]

It's needed for tests, that's what TESTS_ENVIRONMENT is for. IIUC, it only needs to be removed in guile.am, you don't need to remove ./env itself.

The *-impl.scm + override/ machinery didn't exist prior to #77; it is not "needed for tests". My understanding is that it was introduced to support cross-compilation without having that extra FIBERS_CROSS_COMPILING environment variable---a laudable goal, but it failed to provide both.

I don't recall what I was thinking about when I wrote that message, but after-the-fact I would say 'it = ./env', not 'it = PR #77'. I don't recall what testing I did at the time (I did some testing, but I don't recall to what extent).

Maybe try removing './env' from guile.m4 first?

./env sets up the .scm and .go and .so search paths, so I think it's good to use it when running guild compile; otherwise we'd have to basically duplicate these environment variable settings in the .scm.go target. Or am I misunderstanding something?

The .scm search path is already set up by the makefile (-L "$(abs_top_srcdir)$") (*). Though, I suppose you could let a ./env equivalent do the job instead.
To my knowledge, the .so search path is unused, instead it links to .so libraries by file name (so
the modification of LTDL_LIBRARY_PATH and GUILE_EXTENSIONS_PATH could likely be removed).

(*) actually only abs_top_srcdir, not build directory, so slight changes may be needed for out-of-tree compilation.

When compiling, the build directory should be mentioned to Guile (for .go), but currently it can't properly deal with that when cross-compiling, because:

• it conflates '.go to load' with '.go for inlining' (both are GUILE_LOAD_COMPILED_PATH)
• that wouldn't be a problem if the .go included target information so Guile can verify what its purpose is (and we ignored the cross-distro case), but it only records endianness and word size (often, but not always sufficient)
• inlining is disabled when cross-compiling (IIRC because of the above), but even so the problem remains that it would load .go for the wrong architecture or system when endianness & word size matches (probably not yet a problem with current implementation of Fibers and Guile, but do you really want to rely on that?)
• and, even if it could be relied upon for this particular case, note that a method of starting new Guile projects is to copy some Makefile things from another project, and quite possibly it might be problem for that other project.

Overall, I would say that it is dangerous to conflate load paths (.scm for interpreter, .go for JIT) with source/compilation path (.scm to compile, .go to inline), so './env' shouldn't do the job of both. (I think usually the .scm paths are fine to conflate because they are sources, but even then, they aren't always target-independent, e.g. embedding .so file names.)

In a future where Guile properly separates '.go to load' with '.go for inlining', -C build dir could be (re-)added, but it needs some nuance. For example, each file A.scm would correspond to two .go: current-system/A.go and target/A.go. A.scm would be in both .scm paths (unless it's a target-dependent .scm, in which case there are two A.scm, in different paths). current-system would be in .scm for interpreter(& auto-compile)' and target would be in '.go for inlining'. When A.scm depends on macros from B.scm, Guile loads current-system/B.go to get those macros.

[emixa-d]

Also, I would rather avoid setting GUILE_LOAD_COMPILED_PATH until dependency tracking is added (**). E.g., that thing I wrote for Guix proprietary (*) compilation mechanism, but adapted for makefiles and I can't find again on issues.guix.gnu.org.

(It was rejected for being Guix-specific, but that makes little sense because Guix' compilation tool is Guix-specific, of course a dependency tracking mechanism is entwined with the compilation tool, and the reviewer doesn't apply this requirement to their own code (e.g. changes to the very same compilation tool and (guix records).)

(*) the other meaning, not the free/non-free meaning.
(**) see e.g. the issue mentioned in Guix' compilation tool or potential determinism issues (e.g. one compilation run interprets a dependency, so no inlining, another compiles the dependency, so some inlining)