Hi Ryan,
On Wed, 18 Aug 2021 at 03:57, Ryan Prior <rpr...@protonmail.com> wrote:
> I learned today that Guix will chug happily along applying a transform to a
> nonexistent package.
>
> For example, I can run:
> guix environment --with-latest=not-exist --ad-hoc which
>
> This shows no warning or errors. I think it would be beneficial to show an
> error & bail if the target of a transformation is a package that doesn't
> exist, as this is likely indicative of an error.
Indeed. :-) The issue comes from guix/transformations.scm
(options->transformation):
--8<---------------cut here---------------start------------->8---
(let ((new (transform obj)))
(when (eq? new obj)
(warning (G_ "transformation '~a' had no effect on ~a~%")
name
(if (package? obj)
(package-full-name obj)
obj)))
new)
--8<---------------cut here---------------end--------------->8---
and the warning is not reach because guix/packages.scm (package-mapping):
--8<---------------cut here---------------start------------->8---
(else
;; Return a variant of P with PROC applied to P and its explicit
;; dependencies, recursively. Memoize the transformations.
Failing
;; to do that, we would build a huge object graph with lots of
;; duplicates, which in turns prevents us from benefiting from
;; memoization in 'package-derivation'.
(let ((p (proc p)))
(package
…
--8<---------------cut here---------------end--------------->8---
In the case of “guix build hello --with-latest=foo”, then ’proc’ has no
effect, i.e., ’p’ is identical to ’(proc p)’ but a new package is
allocated which leads that ’new’ and ’obj’ are not ’eq?’.
Well, I have tried the attached patch. But then ’tests/packages.scm’
fails. Hum, I need an input because I do not know if I miss something
or if the test is also inaccurate.
--8<---------------cut here---------------start------------->8---
(test-assert "package-input-rewriting/spec"
(let* ((dep (dummy-package "chbouib"
(native-inputs `(("x" ,grep)))))
(p0 (dummy-package "example"
(inputs `(("foo" ,coreutils)
("bar" ,grep)
("baz" ,dep)))))
(rewrite (package-input-rewriting/spec
`(("coreutils" . ,(const sed))
("grep" . ,(const findutils)))
#:deep? #f))
(p1 (rewrite p0))
(p2 (rewrite p0)))
(and (not (eq? p1 p0))
(eq? p1 p2) ;memoization
--8<---------------cut here---------------end--------------->8---
I miss why ’p1’ should not be the same as ’p0’.
> What do you think?
Maybe, it could be worth to open a report for that. Feel free. ;-)
Cheers,
simon
>From e1cd54f4cccad37f7134b342c8dee9da9fa28588 Mon Sep 17 00:00:00 2001
From: zimoun <zimon.touto...@gmail.com>
Date: Wed, 25 Aug 2021 17:32:58 +0200
Subject: [PATCH 1/1] packages: 'package-mapping' does not allocate unwritten
package.
Reported by Ryan Prior <rpr...@protonmail.com>.
* guix/packages.scm (package-mapping): Do not allocate a new package if the
procedure has no effect.
---
guix/packages.scm | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/guix/packages.scm b/guix/packages.scm
index c825f427d8..15aa67fe0a 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2017, 2019, 2020 Efraim Flashner <efr...@flashner.co.il>
;;; Copyright © 2019 Marius Bakke <mba...@fastmail.com>
;;; Copyright © 2021 Chris Marusich <cmmarus...@gmail.com>
+;;; Copyright © 2021 Simon Tournier <zimon.touto...@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -1058,20 +1059,22 @@ applied to implicit inputs as well."
;; to do that, we would build a huge object graph with lots of
;; duplicates, which in turns prevents us from benefiting from
;; memoization in 'package-derivation'.
- (let ((p (proc p)))
- (package
- (inherit p)
- (location (package-location p))
- (build-system (if deep?
- (build-system-with-package-mapping
- (package-build-system p) rewrite)
- (package-build-system p)))
- (inputs (map rewrite (package-inputs p)))
- (native-inputs (map rewrite (package-native-inputs p)))
- (propagated-inputs (map rewrite (package-propagated-inputs p)))
- (replacement (and=> (package-replacement p) replace))
- (properties `((,mapping-property . #t)
- ,@(package-properties p)))))))))
+ (let ((new (proc p)))
+ (if (eq? new p)
+ p
+ (package
+ (inherit new)
+ (location (package-location new))
+ (build-system (if deep?
+ (build-system-with-package-mapping
+ (package-build-system new) rewrite)
+ (package-build-system new)))
+ (inputs (map rewrite (package-inputs new)))
+ (native-inputs (map rewrite (package-native-inputs new)))
+ (propagated-inputs (map rewrite (package-propagated-inputs new)))
+ (replacement (and=> (package-replacement new) replace))
+ (properties `((,mapping-property . #t)
+ ,@(package-properties new))))))))))
replace)
--
2.32.0
