Hi Ludovic, [email protected] (Ludovic Courtès) writes:
> Mark H Weaver <[email protected]> skribis: > >> [email protected] (Ludovic Courtès) writes: >>> Honestly, I wouldn’t worry about the propagation of $GUILE_LOAD_PATH & >>> co. to subprocesses, because we know there’s none anyway. >> >> That policy will lead to future where libguile-using programs break in >> random ways when they happen to be subprocesses of each other. > > I agree in general with your feeling. > > However, in that case, we know that these command-line tools are just > wrappers around our Scheme APIs, and that they won’t ever launch any > program (programs are a thing of the past; procedures are the future). > So it just seemed safe to me to do that in this particular case. > > What do you think? So I've been working on a patch to fix the ./pre-inst-env problem using portable shell code instead of Guile code, as you suggested, and this is the kind of code I'm coming up with: --8<---------------cut here---------------start------------->8--- #!/bin/sh # aside from this initial boilerplate, this is actually -*- scheme -*- code prefix="@prefix@" datarootdir="@datarootdir@" main='(module-ref (resolve-interface '\''(guix-package)) '\'guix-package')' if test "x$GUIX_UNINSTALLED" = x; then GUILE_LOAD_COMPILED_PATH="@guilemoduledir@:$GUILE_LOAD_COMPILED_PATH" export GUILE_LOAD_COMPILED_PATH exec ${GUILE-@GUILE@} -L "@guilemoduledir@" -l "$0" \ -c "(apply $main (cdr (command-line)))" "$@" else exec ${GUILE-@GUILE@} -l "$0" \ -c "(apply $main (cdr (command-line)))" "$@" fi !# --8<---------------cut here---------------end--------------->8--- or perhaps you'd prefer something more like this: --8<---------------cut here---------------start------------->8--- #!/bin/sh # aside from this initial boilerplate, this is actually -*- scheme -*- code prefix="@prefix@" datarootdir="@datarootdir@" if test "x$GUIX_UNINSTALLED" = x; then GUILE_LOAD_COMPILED_PATH="@guilemoduledir@:$GUILE_LOAD_COMPILED_PATH" export GUILE_LOAD_COMPILED_PATH load_path_args="-L @guilemoduledir@" else load_path_args="" fi main='(module-ref (resolve-interface '\''(guix-package)) '\'guix-package')' exec ${GUILE-@GUILE@} $load_path_args -l "$0" \ -c "(apply $main (cdr (command-line)))" "$@" !# --8<---------------cut here---------------end--------------->8--- but the more I look at this ugly, buggy code; and the more I fret about the inherent bugs having to do with poor handling of shell meta-characters and colons in file names; and the more I read of the "Portable Shell Programming" chapter of the autoconf manual, the less I understand why you feel so strongly about using this awful language instead of the Guile code I wrote. To save a few lines? Please take a look at my proposed code one more time with fresh eyes: --8<---------------cut here---------------start------------->8--- #!/bin/sh # aside from this initial boilerplate, this is actually -*- scheme -*- code script=guix-build prefix="@prefix@" datarootdir="@datarootdir@" startup=" (let () (define-syntax-rule (push! elt v) (set! v (cons elt v))) (define (main interpreter module-dir script-file . args) (unless (getenv \"GUIX_UNINSTALLED\") (push! module-dir %load-path) (push! module-dir %load-compiled-path)) (load script-file) (let ((proc (module-ref (resolve-interface '($script)) '$script))) (apply proc args))) (apply main (command-line))) " exec "${GUILE-@GUILE@}" -c "$startup" "@guilemoduledir@" "$0" "$@" --8<---------------cut here---------------end--------------->8--- Allow me to list the advantages: * Schemers will have a much easier time understanding precisely what this code does, without having to grok the finer details of shell programming and Guile's command-line handling. * It sets a good example for how to write a Guile program that will be robust in the case where subprocesses might also use Guile. * The boilerplate code is identical in all scripts except on line 4 (script=guix-build). * It is completely robust in its handling of unusual characters, with the sole exception of "${GUILE-@GUILE@}" which could fail if @GUILE@ contains meta-characters. I could fix that too with a few more lines of code. (And yes, I know that autoconf is already unable to handle filenames with meta-characters, but that's no excuse to create similar bugs in our own code if we can easily avoid it. Besides, maybe some day autoconf can be made more robust). and the only disadvantage I'm aware of: * It's four lines longer (two of which could be trivially eliminated by removing the "script=guix-build" line and instead replacing the two occurrences of "$script" with "guix-build"). I would urge you to please reconsider your position. If you still prefer the shell-based approach, then could you please take care of fixing the ./pre-inst-env bug as you think is best? I don't want my name associated with it. > (BTW, rather than $GUIX_UNINSTALLED, it just occurred to me that > $GUIX_LOAD_PATH would do just as well while being more generic and > easier to implement/use.) I thought about this too, but it seems to me that it wouldn't work properly for "./pre-inst-env guile". Or am I missing something? Regards, Mark
