[bug#47180] [PATCH] gnu: racket: Don't inject store paths into Racket files.

[Philip McGrath]

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))