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

Apparently, during grafting, Guix can somehow mangle compiled

Racket CS files (.zo) such that Racket will refuse to load them.

(Maybe it has something to do with compression?)

So, we stop patching Racket sources with absolute paths to store

files (i.e. for foreign libraries to dlopen).

Instead, we put them in a data file that doesn't get compiled or,

in one case, embed it in C.

Fixes https://issues.guix.gnu.org/47064

Adds a special case at the C level, controlled by a preprocessor macro,

to handle attempts to execute "/bin/sh".

(racket)[arguments](#:configure-flags): Add the CPP flag to enable it.

(racket)[arguments](#:modules): Use srfi-1.

(racket)[arguments](#:phases): Remove 'patch-/bin/sh and 'pre-configure.

Change 'pre-configure-minimal to just change directory.

Add 'patch-config.rktd-lib-search-dirs after 'build and before 'install

to configure Racket's "lib-search-dirs".

(racket, racket-minimal)[inputs]: Add bash-minimal as an explicit input.

(racket-minimal)[source]: Adjust to inherit patches from racket.

---

gnu/local.mk | 2 +

.../patches/racket-sh-via-rktio.patch | 91 +++++++++

gnu/packages/scheme.scm | 190 ++++++++----------

3 files changed, 179 insertions(+), 104 deletions(-)

create mode 100644 gnu/packages/patches/racket-sh-via-rktio.patch

Wow, that was speedy. Thanks for the care you've taken thinking about these

issues. I've tested applying and building this patch and it works!

(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 don't think I know enough about Guix or Racket internals to give it a

proper review or judge whether this will be robust against future grafting

problems, but I do have a few comments

`git am` complained about the following, but I can't spot them looking at

the patch.

.git/rebase-apply/patch:83: trailing whitespace.

.git/rebase-apply/patch:100: trailing whitespace.

.git/rebase-apply/patch:122: trailing whitespace.

--

.git/rebase-apply/patch:124: new blank line at EOF.

On Mon, 15 Mar 2021, Philip McGrath wrote:

[…]

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.

Again, I might be wrong by preferring to always avoid /bin/sh even when

it's available. Hopefully someone more knowledgeable will come along.

Best,

Jack

Hi,

On 3/16/21 1:43 AM, Jack Hill wrote:

Glad to hear it!

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.

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.

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]:

On Tue, 16 Mar 2021, Philip McGrath wrote:

I've opened #47204 for the fonts issue. I haven't done any investigation

yet, and racket certainly wouldn't be the only package to run across fonts

issues with Guix :)

[0] https://issues.guix.gnu.org/47204

I'm happy to help test things, that's what's nice about working on a

community project.

Indeed, and many more reasons.

I had assumed it was actually a problem with adding inadvertent whitespace

to the patch rather than how it was sent. However, since I couldn't

actually find the extra spaces anywhere, I'm not too worried about it.

Other than the warnings it applied cleanly.

I hadn't considered raco distribute (I guess I'm spoiled by `guix pack`).

Your proposed change addresses my concern.

Thanks!

Jack

Apparently, during grafting, Guix can somehow mangle compiled

Racket CS files (.zo) such that Racket will refuse to load them.

(Maybe it has something to do with compression?)

So, we stop patching Racket sources with absolute paths to store

files (i.e. for foreign libraries to dlopen).

Instead, we put them in a data file that doesn't get compiled or,

in one case, embed it in C.

Fixes https://issues.guix.gnu.org/47064

Adds a special case at the C level, controlled by a preprocessor macro,

to handle attempts to execute "/bin/sh".

(racket)[arguments](#:configure-flags): Add the CPP flag to enable it.

(racket)[arguments](#:modules): Use srfi-1.

(racket)[arguments](#:phases): Remove 'patch-/bin/sh and 'pre-configure.

Change 'pre-configure-minimal to just change directory.

Add 'patch-config.rktd-lib-search-dirs after 'build and before 'install

to configure Racket's "lib-search-dirs".

(racket, racket-minimal)[inputs]: Add bash-minimal as an explicit input.

(racket-minimal)[source]: Adjust to inherit patches from racket.

(racket-minimal)[arguments]: Inherit from racket: changes no longer needed.

---

gnu/local.mk | 2 +

.../patches/racket-sh-via-rktio.patch | 87 ++++++++

gnu/packages/scheme.scm | 191 ++++++++----------

3 files changed, 176 insertions(+), 104 deletions(-)

create mode 100644 gnu/packages/patches/racket-sh-via-rktio.patch

Hi Philip,

Philip McGrath <philip@philipmcgrath.com> skribis:

If those files are compressed, and if a store file name survives despite

compression, then grafting can patch it, which could lead to checksum

mismatches or similar.

What error message does Racket produce?

That solves the problem for Racket itself, but wouln’t Racket libraries

have the same issue?

Would it be an option to instead turn off compression and keep doing

things as usual?

Thanks for looking into it, and sorry for the delay!

Ludo’.

Hi!

On 4/10/21 4:59 PM, Ludovic Courtès wrote:

Yes, that's what seems to be happening.

The first error I heard of (and reproduced) was reported by Jack in

https://issues.guix.gnu.org/47064:

```

$bytevector-uncompress: internal error uncompressing

#"\0\0\0\0chez\310\224\206:\r()#\201\256R-d\205\233\24\363\5\20\201P\6A\v\300\0\16\f\6\31\2\f\6\f&H\275\0\1\0\362\bA\377e\0\1\0C\6A\21\3\v\300\0\201\265!\f\6\n\0\a\1\35\0\1+\0\360\27\201\375\300\0\0\0\17\205\210Z\0\0M\215\245\b\4\0\0M9fH\17\206fZ\0\0I\2...

context...:

body of

"/gnu/store/mmrax3f1vx3c8ih9hhgffpvka6chk96w-racket-8.0/share/racket/pkgs/gui-lib/mred/private/wx/gtk/utils.rkt"

body of

"/gnu/store/mmrax3f1vx3c8ih9hhgffpvka6chk96w-racket-8.0/share/racket/pkgs/gui-lib/mred/private/wx/platform.rkt"

```

The error message is referring to an internal Chez Scheme primitive.

The report at https://issues.guix.gnu.org/47614 seems to me to be an

alternative manifestation of the same problem: the hexdump there is

useful and explains why I did see some of the store path when I had

attempted to investigate using `strings`.

So there seem to be at least three bad cases:

1. The grafter can mangle .zo files so that Racket can't

read them at all.

2. The grafter can miss store references in .zo files, so Racket could

end up using the ungrafted versions.

3. With a garbage collection, Racket could try to use the ungrafted

versions but fail to find them at runtime.

Because the problems are triggered by grafting, they only affect

packages that have been patched by Guix. For now, Guix doesn't have the

ability to install more Racket packages. In the longer term, the

one-sentence answer is that it's always possible we might find and

switch to a better approach, but this seems most promising. (A bit more

of my current thinking on that toward the end.)

In theory, this should be possible. I see two significant downsides:

1. Compiled code would be much larger—maybe twice as big—and, if I

recall correctly, load times would be worse, too. With the move to

Racket CS, existing Racket code moved from a world of small and

cheap bytecode to a world of machine code: the default compression

settings have been tuned to avoid an unacceptable worsening of

binary size and load time.

2. Racket very intentionally does not specify the format of zo files,

and indeed the details routinely change: I think there have now

been

14 such changes on Git since the 8.0 release in February. Continuing

to patch zo files sets us up for future breakage, and it seems like

it would be especially easy for maintainers of the Guix package to

miss the implications of such changes to low-level implementation

details (as I did!). For example, Chez Scheme seems to make

compression options programmer-configurable even within a single

object file: if Racket exposed such options, we could well end up

here again.

More broadly, I think the best strategy for Racket packaging will be, as

much as possible, to use Racket's supported configuration features

rather than Guix-specific hacks. This seems especially viable since

Racket has been willing to accept unobtrusive patches upstream that help

things go smoothly for Guix, e.g. with 8.1 we should no longer need any

patches to the build scripts: we're all friendly, parentheses-loving folks.

For another example, it looks like existing

"racket-store-checksum-override.patch" fixes a previous issue discussed

in https://issues.guix.gnu.org/30680 caused by grafting zo files: I

hope this change will also let us remove that patch, though I'd want to

test more before proposing we drop it. So these problems aren't

fundamentally new; they just have an additional symptom since the change

to Racket CS by default in Racket 8.0. If we can fix the root problem of

violating Racket's assumptions by patching zo files, we should be able

to stop hunting down symptoms.

Rather than using "config.rktd", an alternative approach would be to set

things up so that `dlopen` would find the foreign libraries, perhaps via

`LD_LIBRARY_PATH`. This has some intriguing possibilities: I could

imagine Guix providing an alternate `dlopen` implementation that might

be useful beyond Racket.

Nix apparently configures some things via `LD_LIBRARY_PATH`, but their

approach (as I understand it) relies on generating Racket scripts around

all Racket-generated executable, which causes other problems. There

should be workarounds, but it seems better to avoid going down that road

if we can.

Finally, here's a sketch of how `guix import racket` and such might

work. Racket's package system has a concept of "package scope", so that

"installation" scope can coexist with narrower scopes (mostly per-user

scope, though there are more complex possibilities). Right now, Guix

puts installation scope in the read-only store, which basically

corresponds to how other package managers put it in root-owned places,

except Guix can't write to the store to install additional packages. I'm

still at the information-gathering stage, but my current thinking is

that the hypothetical `racket-build-system` should basically take the

source package and turn it into what Racket calls a "built" package

ready to be installed in `static-link` mode, which includes compiling

the code and building the docs (which can involve quite a lot, e.g. ray

tracing icons). Then a profile hook could knit together all of the

Racket packages into an installation package scope. For packages that

depend on foreign libraries, this would be a chance for Guix to add the

necessary paths to the "config.rktd" for the installation.

-Philip

Hi Philip,

Philip McGrath <philip@philipmcgrath.com> skribis:

Yes, similar to the problems observed with SBCL:

https://issues.guix.gnu.org/33848

Interesting (I’m curious how load time can be improved by (1) reading

files in memory instead of merely mmap’ing, and (2) decompressing.)

[...]

That makes sense to me, I’m all for uniting with parentheses-loving

folks of the world. :-)

OK.

I’m testing v2 of the patch and will push shortly if everything goes

well.

What would that alternate dlopen do? It still has to know where to look

for things, somehow.

Yeah, as a rule of thumb I’d say: don’t fiddle with LD_LIBRARY_PATH.

I’m not sure I follow the details and I’m glad you have a plan.

Finally having a Racket importer would be sweet!

Thanks!

Ludo’.

Hi Philip,

Philip McGrath <philip@philipmcgrath.com> skribis:

Yes, similar to the problems observed with SBCL:

https://issues.guix.gnu.org/33848

Interesting (I’m curious how load time can be improved by (1) reading

files in memory instead of merely mmap’ing, and (2) decompressing.)

[...]

That makes sense to me, I’m all for uniting with parentheses-loving

folks of the world. :-)

OK.

I’m testing v2 of the patch and will push shortly if everything goes

well.

What would that alternate dlopen do? It still has to know where to look

for things, somehow.

Yeah, as a rule of thumb I’d say: don’t fiddle with LD_LIBRARY_PATH.

I’m not sure I follow the details and I’m glad you have a plan.

Finally having a Racket importer would be sweet!

Thanks!

Ludo’.

Philip McGrath <philip@philipmcgrath.com> skribis:

Pushed as 834aa48504a24f0c79e858fc295edbf63815a408.

Thanks!

Ludo’.

Hi,

On 4/12/21 8:55 AM, Ludovic Courtès wrote:

This was a very fuzzy thought with a lot of reliance on "somehow"—I'm

not certain it would actually make sense or even be possible—but what I

had in mind is that `dlopen`, together with the dynamic linker and its

various configuration and cache files, has some places it will search

for shared libraries, e.g. in "/lib" and "/usr/lib". If

`gnu-build-system` could somehow communicate with those mechanisms, then

packages doing things like `dlopen("libm.so.6", RTLD_LAZY)` wouldn't

need to have their source code rewritten to include store files: Guix

would arrange for `dlopen` to find "libm.so.6" among the package inputs.

Then Guix would only need to know how to graft whatever mechanism it

used to configure things for `dlopen`, rather than having to worry about

all of the strange things compilers might do with string literals.

But I don't have much of an idea of what "somehow" might be, and I'm not

really a C hacker when I can avoid it :)

-Philip

Hi,

Philip McGrath <philip@philipmcgrath.com> skribis:

To me, the most practical way it can work is by explicitly replacing

“libfoo.so” with “/gnu/store/…/libfoo.so” in the source.

We could change the ‘dlopen’ implementation altogether so that it (say)

looks for a meta-data file listing search paths, but that’s the kind of

intrusive change I’d rather avoid.

Also, there are cases where we actually want “dynamic binding” (search

for the lib and load it *if* it’s available) rather than “static

binding” (hard-code the name of the library so it’s always found when we

dlopen it). This is the case for plugins and such.

I guess we need to explore the problem space a bit further. :-)

Thanks,

Ludo’.