[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/1] packages: Support for full Guix specification
From: |
Ricardo Wurmus |
Subject: |
Re: [PATCH v3 1/1] packages: Support for full Guix specification |
Date: |
Sun, 22 May 2022 08:43:03 +0200 |
User-agent: |
mu4e 1.6.10; emacs 28.0.50 |
Hi Olivier,
thanks for the new patch and my apologies for the delay!
It looks fine, but I can’t help but feel a little confused about what
variables are actual package values and what are wrappers. Sometimes we
have a variable called “packages”, but it’s seemingly always going to be
package wrappers, so we map “package-unwrap”.
I wonder if there’s a way to hide this machinery somewhat. On the other
hand, “package-unwrap” is a no-op for actual packages, so it doesn’t
really matter.
> +(define package-native-inputs
> + (match-lambda
> + ((? package? pkg)
> + (package-native-inputs pkg))
> + ((? inferior-package? pkg)
> + (inferior-package-native-inputs pkg))
> + ((? package-wrapper? pkg)
> + (package-native-inputs (package-wrapper-package pkg)))))
I’m confused about the naming. Is this a recursive application of
PACKAGE-NATIVE-INPUTS? Or are these two different implementations of
PACKAGE-NATIVE-INPUTS, one from GWL and the other from Guix?
> (define (lookup-package specification)
> (log-event 'guix (G_ "Looking up package `~a'~%") specification)
> - (match (lookup-inferior-packages (current-guix) specification)
> - ((first . rest) first)
> - (_ (raise (condition
> - (&gwl-package-error
> - (package-spec specification)))))))
> + (let-values (((name version output)
> + (package-specification->name+version+output
> + specification)))
> + (let* ((inferior-package
> + (lookup-inferior-packages (current-guix)
> + name version))
> + (package (match inferior-package
> + ((first . rest) first)
> + (_ (raise (condition
> + (&gwl-package-error
> + (package-spec specification))))))))
> + (make-package-wrapper package output))))
I think this would be slightly improved by using SRFI-71 instead of
LET-VALUES. SRFI-71 replaces LET and LET* so that you can assign
multiple values without needing to be explicit about it.
Since this procedure is now a bit more complicated I think it would be
good to have a docstring that mentions the input and output values.
> (define default-guile
> (mlambda ()
> "Return the variant of Guile that was used to build the \"guix\"
> package, which provides all library features used by the GWL. We use
> this Guile to run scripts."
> - (and=> (assoc-ref (inferior-package-native-inputs (build-time-guix))
> + (and=> (assoc-ref (package-native-inputs (build-time-guix))
> "guile") first)))
Is this correct? BUILD-TIME-GUIX has been changed so that the return
value is an unwrapped wrapper — so it really is an inferior package.
Does PACKAGE-NATIVE-INPUTS refer to the implementation here or that in
Guix?
I think it would be best if we can make all this unambiguous.
--
Ricardo
- Re: [PATCH v3 1/1] packages: Support for full Guix specification,
Ricardo Wurmus <=