with-parameters does not work generally for packages

  • Open
  • quality assurance status badge
Details
2 participants
  • David Elsing
  • Ludovic Courtès
Owner
unassigned
Submitted by
David Elsing
Severity
important

Debbugs page

David Elsing wrote 1 months ago
(address . bug-guix@gnu.org)
7yikq1p9g4.fsf@posteo.net
Hello,

I noticed that 'with-parameters' from (guix gexp) does not work with
Guile parameters used in package definitions. They are still set
in 'lower-object', but not anymore when the monadic procedure returned
by 'lower-object' is evaluated.

Attached is an example for a package wrapped by 'with-parameters', which
results in a file with "D" instead of "C" (it is not "A", because the
'arguments' field of <package> is thunked).

Is this intentional? I'm not really sure how (or whether) this should be
changed though.

Best,
David
(use-modules (guix build-system trivial) (guix gexp) (guix derivations) (guix packages) (guix store) (gnu packages base)) (define %param (make-parameter "A")) (define testp (package (name "testp") (version "0") (source #f) (build-system trivial-build-system) (arguments (list #:builder #~(let ((port (open-file (string-append #$output) "w"))) (display (string-append #$(%param) "\n") port) (close-port port)))) (home-page #f) (synopsis #f) (description #f) (license #f))) (%param "B") (define obj (with-parameters ((%param "C")) testp)) (%param "D") obj
David Elsing wrote 1 months ago
Re: [bug#70895] [PATCH] grafts: Only compute necessary graft derivations.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 75879@debbugs.gnu.org)
86frky7fjc.fsf@posteo.net
Hello,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (4 lines)
> Uh, looks like this is a real bug. I’m surprised because we do have
> tests for that in ‘tests/gexp.scm’ (and it’s actually used in a few
> important places), but maybe they’re not exercising the right thing.

Yes indeed, 'with-parameters' is tested for %current-system and
%current-target-system, which are evaluated earlier as a special case in
the gexp-compiler of <parameterized>, and an additional parameter is
only tested by immediately evaluating it.

Best,
David
Ludovic Courtès wrote 1 months ago
control message for bug #75879
(address . control@debbugs.gnu.org)
87v7to8muv.fsf@gnu.org
severity 75879 important
quit
Ludovic Courtès wrote 3 weeks ago
Re: bug#75879: with-parameters does not work generally for packages
(name . David Elsing)(address . david.elsing@posteo.net)
87jz9qx6ud.fsf@gnu.org
Hi David,

David Elsing <david.elsing@posteo.net> skribis:

Toggle quote (12 lines)
> I noticed that 'with-parameters' from (guix gexp) does not work with
> Guile parameters used in package definitions. They are still set
> in 'lower-object', but not anymore when the monadic procedure returned
> by 'lower-object' is evaluated.
>
> Attached is an example for a package wrapped by 'with-parameters', which
> results in a file with "D" instead of "C" (it is not "A", because the
> 'arguments' field of <package> is thunked).
>
> Is this intentional? I'm not really sure how (or whether) this should be
> changed though.

Something just came to mind: the object cache. The cache is keyed by
object + system + target + grafts?; if there’s anything that influences
what the object lowers to, changes are the object->derivation mapping is
already cached and that other thing will be ignored.

That’s a problem generally speaking with using ‘with-parameters’ with
parameters other than ‘%current-system’, ‘%current-target-system’, and
‘%graft?’.

I wonder if it’s the only thing at play here though.

Ludo’.
David Elsing wrote 3 weeks ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
8634gcxurc.fsf@posteo.net
Hi Ludo',

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (5 lines)
> Something just came to mind: the object cache. The cache is keyed by
> object + system + target + grafts?; if there’s anything that influences
> what the object lowers to, changes are the object->derivation mapping is
> already cached and that other thing will be ignored.

Oh that's true, this is an additional problem. Would it make sense to
add the parameters passed to `with-parameters' to the keys used in
`mcached'? Then the parameters would have to be passed around quite a
bit however...
Alternatively, would it be possible to query the fluids with
`current-dynamic-state' instead and use that for the keys in `mcached'?
I don't see any way to inspect a dynamic state though.

Toggle quote (4 lines)
> That’s a problem generally speaking with using ‘with-parameters’ with
> parameters other than ‘%current-system’, ‘%current-target-system’, and
> ‘%graft?’.

I think `with-parameters' only works for `%current-system' and
`%current-target-system', not for `%graft?': When setting `%graft?' to
#f in the second `with-parameters' wrapping in
`graft-derivation/shallow', it still evaluates to the same derivation as
with `%graft?' set to #t.

Toggle quote (2 lines)
> I wonder if it’s the only thing at play here though.

No, attached is a simplified example without a package using a record
(named <test>) with a (thunked) field. Removing the `mcached' calls in
(guix gexp) results in the same derivation. I think this is because
`lower-object' returns a monadic procedure, which does not keep the
fluids set by `with-parameters', so when the (thunked) field is
evaluated, the parameter has changed.

Maybe it would be good to pass the parameters from the gexp compiler of
<parameterized> to `lower-object' as additional argument and use
`with-fluids*' there (just before `lower' is called)? This worked for
the <test> record defined in the attachment, but not for a package, I
guess because of the expander? I'm not sure how to pass the parameters
in that case though.

Best,
David
(use-modules (guix gexp) (guix monads) (guix records) (guix store) (guix utils)) (define-record-type* <test> test make-test test? this-test (field test-field (thunked))) (define* (test->derivation test) (with-monad %store-monad (gexp->derivation "test" #~(let ((port (open-file #$output "w"))) (display (string-append #$(test-field test) "\n") port) (close-port port))))) (define-gexp-compiler (test-compiler (test <test>) system target) (test->derivation test)) (define %param (make-parameter "A")) (define testvalue (test (field (%param)))) (%param "B") (define testvalue2 (with-parameters ((%param "C")) testvalue)) (%param "D") testvalue2
Ludovic Courtès wrote 2 weeks ago
(name . David Elsing)(address . david.elsing@posteo.net)
87h64m5815.fsf@gnu.org
Hi David,

David Elsing <david.elsing@posteo.net> skribis:

Toggle quote (8 lines)
> Oh that's true, this is an additional problem. Would it make sense to
> add the parameters passed to `with-parameters' to the keys used in
> `mcached'? Then the parameters would have to be passed around quite a
> bit however...
> Alternatively, would it be possible to query the fluids with
> `current-dynamic-state' instead and use that for the keys in `mcached'?
> I don't see any way to inspect a dynamic state though.

Problem is that the key must remain “small” so that computing its hash
is fast. It cannot grow further than its current size, I’m afraid.

Ideally the key would contain only parameters known to be influential,
but we cannot tell which are influential and which are not among those
in the current dynamic state.

Toggle quote (6 lines)
> I think `with-parameters' only works for `%current-system' and
> `%current-target-system', not for `%graft?': When setting `%graft?' to
> #f in the second `with-parameters' wrapping in
> `graft-derivation/shallow', it still evaluates to the same derivation as
> with `%graft?' set to #t.

The patch I just sent¹ fixes this issue.

The solution was to do the same as ‘mparameterize’ to ensure that the
extent during which parameters are set is the correct one given this is
in a monadic context.

Let me know what you think!

Thanks,
Ludo’.

¹ “gexp: ‘with-parameters’ properly handles ‘%graft?’.”
I cannot see it yet in Debbugs.
David Elsing wrote 2 weeks ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
86h64hsk0f.fsf@posteo.net
Hi Ludo',

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (3 lines)
> Problem is that the key must remain “small” so that computing its hash
> is fast. It cannot grow further than its current size, I’m afraid.

What if the hash is calculated in `compile-parameterized' instead (as
that is the only supported way to set the parameters) and passed to
`lower-object'?

Toggle quote (4 lines)
> Ideally the key would contain only parameters known to be influential,
> but we cannot tell which are influential and which are not among those
> in the current dynamic state.

Yes that's true, I think the parameters affecting the key should only be
the ones passed to `with-parameters', other parameters would then be
assumed to be constant.

Toggle quote (4 lines)
> The solution was to do the same as ‘mparameterize’ to ensure that the
> extent during which parameters are set is the correct one given this is
> in a monadic context.

I think that's a good idea, but I'm not so convinced it makes sense for
a general monad [1]. An alternative solution [2] using `with-fluids*'
has a different problem however, as it doesn't seem to be compatible
with prompts.

Thanks,
David

?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 75879@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 75879
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help