[bug#47869] [PATCH core-updates] ‘which’ looks in PATH, incorrect when cross-compiling

["Ludovic Courtès" <ludo@gnu.org>]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op di 18-05-2021 om 22:51 [+0200]:

[...]

>> The “correct” way we do it right now is like
>> this:
>> 
>>         (lambda* (#:key outputs inputs #:allow-other-keys)
>>           (let ((out (assoc-ref outputs "out"))
>>                 (curl (assoc-ref inputs "curl")))
>>             (wrap-program (string-append out "/bin/akku")
>>               `("LD_LIBRARY_PATH" ":" prefix (,(string-append curl "/lib"))))
>>             #t))
>> 
>> Here, when cross-compiling, (assoc-ref inputs "curl") returns the target
>> cURL.
>
> This is something that can be fixed on 'core-updates', right?

Yes, it’s a good time for this!  For now, we can even afford world
rebuilds on ‘core-updates’.

>> > From e78d2d8651d5f56afa7d57be78c5cccccebb117a Mon Sep 17 00:00:00 2001
>> > From: Maxime Devos <maximedevos@telenet.be>
>> > Date: Sun, 18 Apr 2021 20:44:28 +0200
>> > Subject: [PATCH 3/7] build: utils: Make inputs of 'wrap-script' explicit.
>> > 
>> > Previously, 'wrap-script' used (which "guile") to determine where to locate
>> > the guile interpreter.  But this is incorrect when cross-compiling.  When
>> > cross-compiling, this would locate the (native) guile interpreter that is
>> > in the PATH, while a guile interpreter for the target is required.
>> > 
>> > Remove the optional #:guile argument which is only used in tests and replace
>> > it with a required 'inputs' argument and adjust all callers.  Write a new
>> > test verifying a guile for the target is located, instead of a native guile.
>> 
>> I think the #:guile argument was a fine interface: clear and
>> to-the-point.  The problem IMO is just that it’s not use where it
>> should.  :-)
>
> It should be used practically everywhere, no? So making it optional doesn't
> make much sense to me when we want to support cross-compilation.

Yes, and I agree that’s a difficulty.

>> >           (add-after 'install 'wrap-executables
>> > -           (lambda* (#:key outputs #:allow-other-keys)
>> > +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> >               (let ((out (assoc-ref outputs "out")))
>> > -               (wrap-script (string-append out "/bin/carla")
>> > +               (wrap-script (string-append out "/bin/carla") inputs
>> >                              `("GUIX_PYTHONPATH" ":" prefix (,(getenv "GUIX_PYTHONPATH"))))
>> 
>> This would become:
>> 
>>   (wrap-script (string-append out "/bin/carla")
>>                `(…)
>>                #:guile (assoc-ref inputs "guile"))
>>
>> WDYT?
>
> Ok, this looks a good interface to me, though I think
> 'wrap-script' will need to be modified. IIRC, #:guile
> must be the full file name of the guile binary and not
> simply /gnu/store/[...]-guile-$VERSION.

Good point.  I think one could write:

  (wrap-script … #:guile (search-input-file inputs "/bin/guile"))

which is more reasonable.

> That should help with the verbosity. The previous code becomes
>
>   (wrap-program (string-append libexec "/dhclient-script")
>      `("PATH" …)
>      #:sh (search-input-file inputs "bin/sh"))
>
> which isn't overly verbose.
>
> This procedure 'search-input-file' would return #f if the input
> was not found, right? I would rather it raises an exception instead.
> There have been some bugs where "#f" was silently written into some file,
> which is unlikely to work well.

Agreed, let’s have ‘search-input-file’ raise an exception if the file
isn’t found.

> For the few cases were the input binary is truly optional,
> we can define a 'search-optional-input-file' procedure.

Let’s ignore that until the problem shows up.

>> > -@c TODO document wrap-program
>> > +@deffn {Scheme Procedure} wrap-program @var{prog} @var{inputs} @var{vars}
>> > +Make a wrapper for @var{prog}.  @var{vars} should look like this:
>> > +
>> > +@lisp
>> > +  '(VARIABLE DELIMITER POSITION LIST-OF-DIRECTORIES)
>>    ^
>> You can remove indentation and use @var instead of capital letters.
>
> @var can be used inside @lisp? Didn't know that.

Yes.

Sorry for the delay, and thanks again!

Ludo’.