From: Mark H Weaver <mhw@netris.org>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: bug-guix@gnu.org
Subject: Re: [PATCH] Improve shell script headers and pre-inst-env handling
Date: Wed, 13 Feb 2013 04:55:05 -0500 [thread overview]
Message-ID: <87k3qcilmu.fsf@tines.lan> (raw)
In-Reply-To: <87y5etnqyj.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Tue, 12 Feb 2013 22:48:52 +0100")
Hi Ludovic,
ludo@gnu.org (Ludovic Courtès) writes:
> Mark H Weaver <mhw@netris.org> skribis:
>
>> ludo@gnu.org (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
next prev parent reply other threads:[~2013-02-13 9:55 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-12 1:45 [PATCH] Improve shell script headers and pre-inst-env handling Mark H Weaver
2013-02-12 2:24 ` Mark H Weaver
2013-02-12 4:36 ` Mark H Weaver
2013-02-12 15:53 ` Ludovic Courtès
2013-02-12 15:56 ` Ludovic Courtès
2013-02-12 18:44 ` Mark H Weaver
2013-02-12 21:48 ` Ludovic Courtès
2013-02-12 22:44 ` Mark H Weaver
2013-02-13 14:42 ` Ludovic Courtès
2013-02-13 9:55 ` Mark H Weaver [this message]
2013-02-13 20:57 ` Ludovic Courtès
2013-02-14 8:28 ` Mark H Weaver
2013-02-14 9:44 ` [PATCH] Replace individual scripts with master 'guix' script Mark H Weaver
2013-02-14 13:41 ` Ludovic Courtès
2013-02-14 23:13 ` Mark H Weaver
2013-02-16 20:57 ` Ludovic Courtès
2013-02-17 14:59 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k3qcilmu.fsf@tines.lan \
--to=mhw@netris.org \
--cc=bug-guix@gnu.org \
--cc=ludo@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).