[bug#43193] [PATCH] guix: Add --with-dependency-source option

[Jesse Gibbons]

On 9/10/20 3:43 AM, Ludovic Courtès wrote:
> Hi Jesse,
>
> Jesse Gibbons <jgibbons2357@gmail.com> skribis:
>
> >      The attached patch adds support for the --with-dependency-source
> > common build option. It can be used to specify the sources of
> > dependencies of specified packages. With this step, we can close
> > #42155. This is also a step in closing #43061.
> Excellent!
>
> > Note that I suggested making a new
> > --with-dependency-source=package=source build option in response to
> > #42155 and nobody responded with an objection. So I am sending this
> > patch with the assumption that there are no objections to this new
> > build option, and that if a member of the community wants to
> > completely replace the behavior of --with-source with the behavior of
> > the new option, that person can do the work required to not break
> > --with-source=source.
> OK.  Like I wrote at <https://issues.guix.gnu.org/42155#3>, I wouldn’t
> mind simply calling this new option ‘--with-source’.  What we’d lose by
> doing so is the warning you get if you do
> ‘--with-source=does-not-exist=whatever’, saying the option had no
> effect, but that’s about it.  The new ‘--with-source’ behavior
> (recursive) would be consistent with the other options, which, to me, is
> more important.
I agree that '--with-source' is a better name for the option, but I 
don't want to lose that particular functionality. I worked a little more 
to alter "--with-source" while still preserving the simple 
'--with-source=source' option, because once it's committed to master, it 
will be difficult to turn back and get the ideal implementation. The 
result is a bit dirty and should be refactored and cleaned, but at least 
it works. Attached is the updated patch.
> WDYT?
>
> > >From 91a89277067fd454ad77edb3a09ed06382f3694c Mon Sep 17 00:00:00 2001
> > From: Jesse Gibbons <jgibbons2357+guix@gmail.com>
> > Date: Thu, 3 Sep 2020 17:45:08 -0600
> > Subject: [PATCH v1 1/1] guix: Add --with-dependency-source option
> >
> > * guix/scripts/build.scm: (transform-package-inputs/source): new
> >    function
> > (evaluate-source-replacement-specs): new function
> > (%transformations): add with-dependency-source option
> > (%transformation-options): add with-dependency-source-option
> > (show-transformation-options-help): document --with-dependency-source
> [...]
>
> > +(define (evaluate-source-replacement-specs specs proc)
> > +  "Parse SPECS, a list of strings like \"guile=/path/to/source\", and return a
> > +list of package pairs, where (PROC PACKAGE URL) returns the replacement 
> > package.
> > +Raise an error if an element of SPECS uses invalid syntax, or if a package it
> > +refers to could not be found."
> > +  (map (lambda (spec)
> > +         (match (string-tokenize spec %not-equal)
> > +           ((spec url)
> > +            (define (replace old)
> > +              (proc old url))
> > +            (cons spec replace))
> > +           (x
> > +            (leave (G_ "invalid replacement specification: ~s~%") spec))))
>                                    ^
> Add “source” here.  It’s always a good idea to not have the exact same
> error message in several places.  :-)
Fixed.
> Could you:
>
>    1. adjust doc/guix.texi accordingly?  That is, if we rename this new
>       option to ‘--with-source’, simply add a note stating that it’s
>       recursive.
I included this in the attached patch.
>    2. add a test to tests/guix-build.sh?  There are already --with-source
>       tests in other files.  You can mimic them, essentially to make sure
>       the option has an effect.
>    3. optionally add an entry as a separate commit to
>       etc/news.scm.  I can do that for you if you want.
Do you still think the tests should be updated and this change should be 
announced in the news file? I'm willing to do these.
> Thanks!
>
> Ludo’.
-Jesse

0001-guix-Make-with-source-option-recursive.patch
Description: Text Data