[bug#55248,2/7] gnu: racket: Fix out-of-source build.

* gnu/packages/racket.scm (racket-vm-cgc)[arguments]: Add a 'symlink-src'
phase to put license files where 'install-license-files' expects to find
them. Supply '#:out-of-source? #t'.
---
 gnu/packages/racket.scm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]:
> -       ;; help with debugging, but it confuses `install-license-files`.
[...]
> +             ;; workaround for install-license-files
> +             (lambda* (#:key out-of-source? #:allow-other-keys)
> +               (when out-of-source?
> +                 (with-directory-excursion ".."
> +                   (symlink "src"
> +                            (package-name->name+version
> +                             (strip-store-file-name #$output))))))))))

Surely we could fix this bug/limitation of install-license-files
upstream (in this case, upstream=Guix)?

Greetings,
Maxime.

Hi,

On 5/4/22 05:29, Maxime Devos wrote:
> Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]:
>> -       ;; help with debugging, but it confuses `install-license-files`.
> [...]
>> +             ;; workaround for install-license-files
>> +             (lambda* (#:key out-of-source? #:allow-other-keys)
>> +               (when out-of-source?
>> +                 (with-directory-excursion ".."
>> +                   (symlink "src"
>> +                            (package-name->name+version
>> +                             (strip-store-file-name #$output))))))))))
> 
> Surely we could fix this bug/limitation of install-license-files
> upstream (in this case, upstream=Guix)?
> 

I'd certainly be in favor of that! I've never edited '(guix build 
gnu-build-system)' but I assume that would be a 'core-updates' change. 
I'm also not sure what the best solution would be in general.

In Racket's specific case, the source directory for the Racket VM is 
(relative to the repository root), "racket/src/", where other notable 
directories present include "racket/collects/" and "pkgs/". An 
out-of-source build ends up happening in "racket/build/". The license 
files are in the source directory, which is what install-license-files 
expects, but the find-source-directory helper function fails to guess 
the location of the source directory:

>   (define (find-source-directory package)
>     ;; For an out-of-source build, guess the source directory location
>     ;; relative to the current directory.  Return #f on failure.
>     (match (scandir ".."
>                     (lambda (file)
>                       (and (not (member file '("." ".." "build")))
>                            (file-is-directory?
>                             (string-append "../" file)))))
>       (()                                         ;hmm, no source
>        #f)
>       ((source)                                   ;only one other file
>        (string-append "../" source))
>       ((directories ...)                          ;pick the most likely one
>        ;; This happens for example with libstdc++, which lives within the GCC
>        ;; source tree.
>        (any (lambda (directory)
>               (and (string-prefix? package directory)
>                    (string-append "../" directory)))
>             directories))))

Some possibilities I can imagine:

   * We could add a case to find-source-directory specifically for "src".

   * We could change configure to communicate the location of the source
     directory, e.g. via an environment variable, rather than trying to
     guess.

   * We could change install-license-files to do more searching in
     general, e.g. trying multiple directories or preferring a directory
     that contains at least one match for #:license-file-regexp. We'd
     have to consider the potential for false positives, too (e.g.
     sibling directories with projects under different licenses.)

   * We could add an argument to install-license-files to specify the
     directory explicitly, as a complement to #:license-file-regexp.

Another scenario that might be worth considering is projects that follow 
the REUSE specification[1], which I don't think would be handled by the 
current install-license-files.

But actually, for Racket, I forgot that a patch I'd sent upstream to 
handle this in `make install` should be in 8.5, so we may not need this 
patch at all: I'll confirm before I send v2.

-Philip

[1]: https://reuse.software/
[2]: https://github.com/racket/racket/pull/4127

Am Donnerstag, dem 05.05.2022 um 14:53 -0400 schrieb Philip McGrath:
> Hi,
> 
> On 5/4/22 05:29, Maxime Devos wrote:
> > Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]:
> > > -       ;; help with debugging, but it confuses `install-license-
> > > files`.
> > [...]
> > > +             ;; workaround for install-license-files
> > > +             (lambda* (#:key out-of-source? #:allow-other-keys)
> > > +               (when out-of-source?
> > > +                 (with-directory-excursion ".."
> > > +                   (symlink "src"
> > > +                            (package-name->name+version
> > > +                             (strip-store-file-name
> > > #$output))))))))))
> > 
> > Surely we could fix this bug/limitation of install-license-files
> > upstream (in this case, upstream=Guix)?
> > 
> 
> I'd certainly be in favor of that! I've never edited '(guix build 
> gnu-build-system)' but I assume that would be a 'core-updates' change.
>  
It would.

> I'm also not sure what the best solution would be in general.
> 
> In Racket's specific case, the source directory for the Racket VM is 
> (relative to the repository root), "racket/src/", where other notable
> directories present include "racket/collects/" and "pkgs/". An 
> out-of-source build ends up happening in "racket/build/". The license
> files are in the source directory, which is what install-license-files
> expects, but the find-source-directory helper function fails to guess
> the location of the source directory:
I think the issue here is that we're in a chdir into the source, where
the typical assumption that there's only a single directory besides it
fails.

> >   (define (find-source-directory package)
> >     ;; For an out-of-source build, guess the source directory
> > location
> >     ;; relative to the current directory.  Return #f on failure.
> >     (match (scandir ".."
> >                     (lambda (file)
> >                       (and (not (member file '("." ".." "build")))
> >                            (file-is-directory?
> >                             (string-append "../" file)))))
> >       (()                                         ;hmm, no source
> >        #f)
> >       ((source)                                   ;only one other
> > file
> >        (string-append "../" source))
> >       ((directories ...)                          ;pick the most
> > likely one
> >        ;; This happens for example with libstdc++, which lives
> > within the GCC
> >        ;; source tree.
> >        (any (lambda (directory)
> >               (and (string-prefix? package directory)
> >                    (string-append "../" directory)))
> >             directories))))
I don't think 
  (lambda (directory)
    (and (string-prefix? package directory)
         (string-append "../" directory)))
does what we think it does.  Even so, it's debatable whether that works
for Racket.  

> Some possibilities I can imagine:
> 
>    * We could add a case to find-source-directory specifically for
>      "src".
Not a fan personally.

>    * We could change configure to communicate the location of the
>      source directory, e.g. via an environment variable, rather than
>      trying to guess.
That would work, but gratuitous environment variables could have
unexpected consequences.  However, note that the build systems
themselves already need to capture the information of where the source
directory lives!  For gnu-build-system, you could check for abs_srcdir
in the Makefile for instance.

If this ever becomes necessary (reading ahead it doesn't seem to be,
but let that if be an if), I'd suggest splitting install-license-files
into a helper procedure that assumes we're in the correct directory and
a top level procedure, that performs the guess.  This top level
procedure would need to be implemented once per build system, while the
helper could be inherited.

>    * We could change install-license-files to do more searching in
>      general, e.g. trying multiple directories or preferring a
>      directory that contains at least one match for
>      #:license-file-regexp. We'd have to consider the potential for
>      false positives, too (e.g. sibling directories with projects
>      under different licenses.)

>    * We could add an argument to install-license-files to specify the
>      directory explicitly, as a complement to #:license-file-regexp.
I would not do either.  An alternative to adding another phase is
wrapping install-license-files in a directory excursion within this
package.  That'd be less code which achieves the same thing.

> Another scenario that might be worth considering is projects that
> follow the REUSE specification[1], which I don't think would be
> handled by the current install-license-files.
I don't think we can rely on projects consistently following them,
sadly.

> But actually, for Racket, I forgot that a patch I'd sent upstream to 
> handle this in `make install` should be in 8.5, so we may not need
> this patch at all: I'll confirm before I send v2.
That is of course the best solution :)

Philip McGrath schreef op do 05-05-2022 om 14:53 [-0400]:
> Another scenario that might be worth considering is projects that follow 
> the REUSE specification[1], which I don't think would be handled by the 
> current install-license-files.

It doesn't copy .reuse/dep5 and LICENSES yet, though that can be
implemented: <https://issues.guix.gnu.org/54234#4>.  I don't think that
helps for this particular package though, given that Racket doesn't
ship those files.

Greetings,
Maxime.

Liliana Marie Prikler schreef op do 05-05-2022 om 21:52 [+0200]:
> > Another scenario that might be worth considering is projects that
> > follow the REUSE specification[1], which I don't think would be
> > handled by the current install-license-files.
> I don't think we can rely on projects consistently following them,
> sadly.

There's plenty of projects that don't follow REUSE (e.g., Guix itself),
and there might be some that have the .reuse/dep5 and LICENSES but
aren't doing things 100% according to the spec, but I don't think it
matters much to Guix if SPDX-License-Identifier is present in every
file and such, given that all Guix has to do here is copy license
files, so Guix could be taught to copy .reuse/dep5 and LICENSES (in
addition to LICENSE, COPYING, ...).

Greetings,
Maxime

On 5/5/22 16:33, Maxime Devos wrote:
> Philip McGrath schreef op do 05-05-2022 om 14:53 [-0400]:
>> Another scenario that might be worth considering is projects that follow
>> the REUSE specification[1], which I don't think would be handled by the
>> current install-license-files.
> 
> It doesn't copy .reuse/dep5 and LICENSES yet, though that can be
> implemented: <https://issues.guix.gnu.org/54234#4>.  I don't think that
> helps for this particular package though, given that Racket doesn't
> ship those files.
> 

Right, it wouldn't help Racket, it was just another example of a case 
not handled by the current implementation. (Though I think it would be 
much more justifiable to add a special case for REUSE than for Racket.) 
Glad people are already thinking about it!

-Philip