[bug#58032] [PATCH] transformations: '--with-source' now operates in depth.

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>> +                   (name (hyphen-package-name->name+version base)))
>> +              (cons name
>> +                    (lambda (old)
>> +                      (package-with-source old uri)))))
>> +           ((spec uri)
>> +            (let-values (((name version)
>> +                          (package-name->name+version spec)))
>
> You usually recommend (srfi srfi-71) when using multiple values; why not
> use it here?  I don't have a preference myself (I find srfi-71
> surprising for the non-initiated (I was), although I like its simple
> interface! :-)).

This file is still on SRFI-11 so I followed that, but now that you
mention it, I’ll switch it to SRFI-71 in a subsequent commit.  :-)

>>  (test-assert "options->transformation, with-source, no matches"
>> -  ;; When a transformation in not applicable, a warning must be raised.
>>    (let* ((p (dummy-package "foobar"))
>>           (s (search-path %load-path "guix.scm"))
>>           (t (options->transformation `((with-source . ,s)))))
>> -    (let* ((port (open-output-string))
>> -           (new  (parameterize ((guix-warning-port port))
>> -                   (t p))))
>> -      (and (eq? new p)
>> -           (string-contains (get-output-string port)
>> -                            "had no effect")))))
>> +    (eq? (package-source (t p))
>> +         (package-source p))))
>
> I'd personally find it a better interface if it failed noisily when
> --with-source doesn't have any effect.  The warning was of little use
> because it got lost in the other outputs; now it would be totally
> silent, right?

Yes, it’ll be silent now, as with the other options.

That’s not great; the problem is that it’s impossible to tell whether a
package variant differs from the original one until they’ve been lowered
to a bag (or a derivation).

>> +(test-assert "options->transformation, with-source, in depth"
>> +  (let* ((p0 (dummy-package "foo" (version "0.0")))
>> +         (s  (search-path %load-path "guix.scm"))
>> +         (f  (string-append "foo@42.0=" s))
>> +         (t  (options->transformation `((with-source . ,f))))
>> +         (p1 (dummy-package "bar" (inputs (list p0))))
>> +         (p2 (dummy-package "baz" (inputs (list p1)))))
>> +    (with-store store
>> +      (let ((new (t p2)))
>> +        (and (not (eq? new p2))
>> +             (match (package-inputs new)
>> +               ((("bar" p1*))
>> +                (match (package-inputs p1*)
>> +                  ((("foo" p0*))
>> +                   (and (not (eq? p0* p0))
>> +                        (string=? (package-name p0*) (package-name p0))
>> +                        (string=? (package-version p0*) "42.0")
>> +                        (string=? (add-to-store store (basename s) #t
>> +                                                "sha256" s)
>> +                                  (run-with-store store
>> +                                    (lower-object
>> +                                     (package-source p0*))))))))))))))
>> +
>
> The recursive? option should probably be #f in the add-store above,
> since the "dummy" source is a single file.

No, it must be #t because that’s also how the transformation interns the
file (the option is called “recursive?” but it has little to do with
recursive directory traversal and more to do with the serialization
format.)

> It may be better to create the dummy file ourselves instead of relying
> on the existence of a 'guix.scm' one (it'd help clarify the test too,
> that bit was a bit mysterious at first).

Right, we could do that but that’s a tiny bit more verbose.  Here I
followed what existing tests do; perhaps we should plan for a face lift
of some of these tests.

Thanks for your feedback!

Ludo’.