From: Philip McGrath <philip@philipmcgrath.com>
To: Jack Hill <jackhill@jackhill.us>
Cc: 47180@debbugs.gnu.org
Subject: [bug#47180] [PATCH] gnu: racket: Don't inject store paths into Racket files.
Date: Tue, 16 Mar 2021 13:37:25 -0400 [thread overview]
Message-ID: <a7e1d021-9b8e-ff99-7731-409fadf6283f@philipmcgrath.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2103160038330.8138@marsh.hcoop.net>
Hi,
On 3/16/21 1:43 AM, Jack Hill wrote:
> I've tested applying and building this patch and it works!
Glad to hear it!
> (Now that DrRacket starts it appears that I may have a font issue with
> it. I'll investigate more an open a separate bug if so.)
I realized to my chagrin when I saw your report that I hadn't actually
used any graphical programs via Guix, which of course would be an
important thing to test! I'll experiment, too, but yes, please do look
for further issues.
A good motivation for `guix import racket` is that we'll be able to run
the test suite, which lives in the `racket-test` package.
> `git am` complained about the following, but I can't spot them looking
> at the patch.
I'm not sure what's up there … I generated it with `git send-email`. But
I'm not very familiar with this workflow, so it's quite possible there's
something I should have done/be doing differently.
>> + 3. "/bin/sh" does not exist (if it does, we don't want to change
>> + the expected behavior); and
>
> Do we really want #3? That would have a different effect than the
> current patching behavior intends. /bin/sh is present by default on Guix
> System, and likely to be present of foreign distros. I think in
> generally we like to avoid this dynamic binding and instead use the sh
> that was included as a package input.
That's a very good question.
The non-obvious scenario I've been considering is that DrRacket's
`Racket|Create Executable…`, `raco distribute`, or the function
`assemble-distribution` from the `compiler/distribute` library can
compile your Racket program and bundle it with its runtime support into
a tarball, which is intended to be portable enough that you can email it
to your friend running some other GNU/Linux distribution and they can
run it, too. One implication of this feature is that a well-behaved
Racket library should cooperate with the compilation manager to register
any foreign shared libraries it may want to bring along.
I don't use this feature much and it would take further testing before
I'd be confident that works properly on Guix, but it was definitely
broken with the old/current way of patching Racket source files with
paths to foreign libraries on the store. Configuring the
`lib-search-dirs` is at least a step closer to The Right Thing.
This patch doesn't try to bring along "sh", and I don't think it should:
the relevant consideration here is that `librktio` will be bundled (IIRC
as part of `libracketcs.a`), so whatever version of `rktio_process` we
compile ought to work without Guix.
That said, I think you may be right that, on Guix, using the sh that was
a package input may be less surprising than considering "/bin/sh" if it
exists. (I also think it's pretty unreasonable to put something other
than a POSIX-compatable shell at "/bin/sh" and expect any good to come
of it.) All the Racket docs[1] promise is that the relevant function
"executes a shell command asynchronously (using sh on Unix and Mac OS,
cmd on Windows)", which I take to mean that our obligation is to provide
a sh, not for it to be any particular sh or a user-configurable sh. (It
does not consult SHELL, for example.)
So, if this still seems right to you, I propose to revise the patch by
dropping condition #3: then we'll still fall back to "/bin/sh" if the
built-in path doesn't exist, as presumably will be the case if we're
being executed in a non-Guix environment, but we'll unconditionally use
the sh that was a package input on Guix.
Thanks for looking over this thoughtfully!
-Philip
[1]:
https://docs.racket-lang.org/reference/subprocess.html#(def._((lib._racket%2Fsystem..rkt)._process))
next prev parent reply other threads:[~2021-03-16 18:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-16 2:56 [bug#47180] [PATCH] gnu: racket: Don't inject store paths into Racket files Philip McGrath
2021-03-16 5:43 ` Jack Hill
2021-03-16 17:37 ` Philip McGrath [this message]
2021-03-17 3:20 ` Jack Hill
2021-03-19 2:34 ` [bug#47180] [PATCH v2] " Philip McGrath
2021-04-12 16:48 ` bug#47180: [PATCH] " Ludovic Courtès
2021-04-10 20:59 ` [bug#47180] " Ludovic Courtès
2021-04-12 3:40 ` Philip McGrath
2021-04-12 12:55 ` Ludovic Courtès
2021-04-12 12:55 ` Ludovic Courtès
2021-04-16 20:16 ` Philip McGrath
2021-04-17 10:12 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a7e1d021-9b8e-ff99-7731-409fadf6283f@philipmcgrath.com \
--to=philip@philipmcgrath.com \
--cc=47180@debbugs.gnu.org \
--cc=jackhill@jackhill.us \
/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 external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.