bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi, On Sat, 21 Oct 2023 at 17:22, Ulf Herrman wrote: >> I don’t know, should we start by having a proper bug report for this and >> study how this happen? [...] >> Again I’m sorry if I’m slow to understand, but I’d like to make sure we >> have a good understanding of the problem before we start discussing >> solutions. > > Okay, I suppose I should have given a concrete example of the behavior. > The qgit example can fill that role: > > $ cat $(./pre-inst-env guix build qgit -n --derivations --no-grafts > --with-latest=qtbase) If I might, Guix revision a25a492f2b, my understanding is, $ ./pre-inst-env guix build qgit -n --with-latest=qtbase /gnu/store/gx5d03as0k1w6jv0pssi6j69n8glf6w5-qgit-2.10.drv /gnu/store/h02aizdjy4p10n4gmcy0y35x14lmjx3n-qtbase-6.6.0.drv Then the builder of the derivation /gnu/store/gx5d03as0k1w6jv0pssi6j69n8glf6w5-qgit-2.10.drv reads, --8<---cut here---start->8--- (begin (use-modules (guix build qt-build-system) (guix build utils)) (qt-build #:source "/gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout" #:system "x86_64-linux" #:outputs (list (cons "out" ((@ (guile) getenv) "out"))) #:inputs (quote (("source" . "/gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout") ("cmake" . "/gnu/store/ygab8v4ci9iklaykapq52bfsshpvi8pw-cmake-minimal-3.24.2") ("qtbase" . "/gnu/store/khlz8afih21pd0szn5x1ck6bp6w320cz-qtbase-6.6.0") [...] #:qtbase "/gnu/store/h8z3xhpb8m1ih2k45935kqx0wya5w7vq-qtbase-5.15.10" --8<---cut here---end--->8--- Therefore, the transformation does not rewrite #:qtbase. And note that qtbase-5.15.10 is not listed as #:inputs. Without the transformations, it reads, --8<---cut here---start->8--- (begin (use-modules (guix build qt-build-system) (guix build utils)) (qt-build #:source "/gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout" #:system "x86_64-linux" #:outputs (list (cons "out" ((@ (guile) getenv) "out"))) #:inputs (quote (("source" . "/gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout") ("cmake" . "/gnu/store/ygab8v4ci9iklaykapq52bfsshpvi8pw-cmake-minimal-3.24.2") ("qtbase" . "/gnu/store/h8z3xhpb8m1ih2k45935kqx0wya5w7vq-qtbase-5.15.10") [...] #:qtbase "/gnu/store/h8z3xhpb8m1ih2k45935kqx0wya5w7vq-qtbase-5.15.10" --8<---cut here---end--->8--- Cheers, simon
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Ludovic Courtès writes: > I don’t know, should we start by having a proper bug report for this and > study how this happen? Does that mean opening a new issue, or what? The bug you've described is the one this issue was initially opened for. > Again I’m sorry if I’m slow to understand, but I’d like to make sure we > have a good understanding of the problem before we start discussing > solutions. Okay, I suppose I should have given a concrete example of the behavior. The qgit example can fill that role: $ cat $(./pre-inst-env guix build qgit -n --derivations --no-grafts --with-latest=qtbase) The way this happens is that qt-build-system's bag-builder (e.g. qt-build, qt-cross-build)) introduces a reference to the qtbase from its #:qtbase argument into the gexp it creates a derivation from, completely independently of any inputs. qgit doesn't explicitly specify #:qtbase, and qt-build-system's 'lower' procedure doesn't add one, so the default value for that keyword argument, (default-qtbase), is used in qt-build. This default value can only be overridden by explicitly passing #:qtbase as a package or bag argument. This requirement doesn't map well to a generic package transformation interface at all - it requires that every transformer (e.g. package-mapping) check if the package it's transforming is using qt-build-system, and if so explicitly supply a #:qtbase that is the result of transforming the original implicit or explicit value, after somehow figuring out what that may be (currently only doable by manually reading guix/build-system/qt.scm). This behavior is also currently exhibited by all build systems' handling of #:guile. Here's a concrete example of that, taken from another mailing I sent to this issue (https://issues.guix.gnu.org/65665#15): - (use-modules (guix packages) (guix profiles) (gnu packages base)) (define guile-named-lyle (package (inherit (default-guile)) (name "lyle"))) ;; contrived example that only replaces hello's immediate dependencies (define hello-transformer (package-mapping (lambda (p0) (if (eq? p0 (default-guile)) guile-named-lyle p0)) (lambda (p) (not (eq? p hello))) #:deep? #t)) (define hello-with-lyle (hello-transformer hello)) (packages->manifest (list hello hello-with-lyle)) ;; $ guix build --derivations --manifest=THISFILE ;; Expectation: build for hello-with-lyle is done with guile-named-lyle. ;; Reality: derivation for hello-with-lyle is the same as hello. - Hopefully that makes it clear why this is happening. As for solutions, some options that come to mind: 1. guile and qtbase, instead of being passed to bag builders as a separate argument, are passed as a bag input that is looked up by magic input label, e.g. "guile-for-build", "qtbase-for-build", etc. Seems fragile, but requires no changes to package-mapping, etc. 2. Modify the build systems so that these kinds of implicit arguments that are packages are always present in bags, and the defaults for those keyword arguments in e.g. qt-build are never used. This is the approach I've taken with https://issues.guix.gnu.org/issue/65665/attachment/4/0/3. This still requires that bag arguments that are packages are transformed in addition to inputs. I hope that counts as a proper bug report. - Ulf signature.asc Description: PGP signature
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi, Ulf Herrman skribis: >> $ ./pre-inst-env guix build qgit -n --with-latest=qtbase >> following redirection to >> `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/'... >> following redirection to >> `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/'... >> guix build: warning: cannot authenticate source of 'qtbase', version 6.6.0 >> >> Starting download of /tmp/guix-file.CTehnY >> From >> https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/6.6.0/submodules/qtbase-everywhere-src-6.6.0.tar.xz... >> …-src-6.6.0.tar.xz 46.1MiB >> >> 12.9MiB/s 00:04 ▕██▏ 100.0% >> substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% >> substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% >> substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... >> 100.0% >> substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... >> 100.0% >> The following derivations would be built: >> /gnu/store/paixxkdaakv55bffggxx4l9hiknl8i5r-qgit-2.10.drv >> /gnu/store/f9fdjk1g1s1aqmlmi4clla2kqns7283v-qtbase-6.6.0.drv >> 0.4 MB would be downloaded: >> /gnu/store/nl9dadzfmjm9wg7v3r31jkx773dl683x-module-import-compiled >> /gnu/store/6zryxmypw0wygayc9pvhyxkx47w0lyci-gperf-3.1 >> /gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout > > And if you do a quick 'cat > /gnu/store/paixxkdaakv55bffggxx4l9hiknl8i5r-qgit-2.10.drv'... [...] > you'll see we have both a > ("/gnu/store/f9fdjk1g1s1aqmlmi4clla2kqns7283v-qtbase-6.6.0.drv",["out"]) > and a > ("/gnu/store/hjhr64r5x3bhdw63zz3a3v09vfrlkhrh-qtbase-5.15.10.drv",["out"]) > in use. > > (and if this were a transformation that applied to all packages, it > would be using two variants of the entire world beneath qtbase also) D’oh! Now we have a bug to chew. (Sorry if this was obvious to you from the start; it wasn’t to me!) I don’t know, should we start by having a proper bug report for this and study how this happen? Again I’m sorry if I’m slow to understand, but I’d like to make sure we have a good understanding of the problem before we start discussing solutions. Thanks, Ludo’.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Ludovic Courtès writes: > Hello, > > Ulf Herrman skribis: > >> That and a growing thirst for a nuclear option for package rewriting >> brought about by trying to debug deep transformations while >> simultaneously experimenting with rewriting a manifest of around 270 >> packages. > > On that topic of a catch-all option for rewriting: there’s an unused > procedure called ‘map-derivation’, which performs rewriting at the level > of derivations. It’s harder to use, more expensive, but who knows, > perhaps we’ll find a motivating use case… (Allowing for graph rewriting > has been a major goal for me since the beginning.) > >> There are really several distinct issues at play here: >> 1. The case of #:qtbase and #:guile being invisible to package-mapping. >>This is what I first noticed, and cannot be fixed without modifying >>the build systems. This is what prompted looking for packages in >>package and bag arguments, and recursing into lists therein (just in >>case an argument took a list of packages). > > How are #:qtbase and #:guile invisible to package mapping? > > They appear in the bag inputs and thus are definitely visible to > ‘package-mapping’, as a I showed with the CMake and Python examples in > this thread. "Invisible to package-mapping" was perhaps not the clearest phrasing; "not completely replaced by package-mapping" might be better. qtbase does indeed go in the bag inputs, but replacing the bag inputs has no effect on the bag arguments, and even if we replaced it in the bag arguments as well, it might not even be in there to begin with, in which case the bag-builder would introduce a default that is only visible at the level of derivations. > > For good measure :-) here’s an example with #:qtbase: > > $ ./pre-inst-env guix build qgit -n > substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% > substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% > 0.4 MB would be downloaded: > /gnu/store/7b20q17yg90b62404chgbnwgvd6ry1qf-qgit-2.10 > $ ./pre-inst-env guix build qgit -n --with-latest=qtbase > following redirection to > `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/'... > following redirection to > `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/'... > guix build: warning: cannot authenticate source of 'qtbase', version 6.6.0 > > Starting download of /tmp/guix-file.CTehnY > From > https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/6.6.0/submodules/qtbase-everywhere-src-6.6.0.tar.xz... > …-src-6.6.0.tar.xz 46.1MiB > > 12.9MiB/s 00:04 ▕██▏ 100.0% > substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% > substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% > substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... > 100.0% > substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... > 100.0% > The following derivations would be built: > /gnu/store/paixxkdaakv55bffggxx4l9hiknl8i5r-qgit-2.10.drv > /gnu/store/f9fdjk1g1s1aqmlmi4clla2kqns7283v-qtbase-6.6.0.drv > 0.4 MB would be downloaded: > /gnu/store/nl9dadzfmjm9wg7v3r31jkx773dl683x-module-import-compiled > /gnu/store/6zryxmypw0wygayc9pvhyxkx47w0lyci-gperf-3.1 > /gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout And if you do a quick 'cat /gnu/store/paixxkdaakv55bffggxx4l9hiknl8i5r-qgit-2.10.drv'... -- Derive ([("out","/gnu/store/z8qrhfmicylxy2mwvcvh9sizfhd3x4d3-qgit-2.10","","")] ,[("/gnu/store/0jk33gpzyicyppbnh458213007v0qjhs-mesa-23.1.4.drv",["out"]) ,("/gnu/store/0njvvgjlb52abhqnmb4rx22sfkxm2h9c-gcc-11.3.0.drv",["out"]) ,("/gnu/store/0p1sns81qbgr8ayiv02fv4rm5drcycd7-libxdamage-1.1.5.drv",["out"]) ,("/gnu/store/14a2ban238fng3c8632lrfkmz54y7m2c-binutils-2.38.drv",["out"]) ,("/gnu/store/1n39zcbr528b7rh9bf1pwfrm7mv8nr8m-bzip2-1.0.8.drv",["out"]) ,("/gnu/store/2pv3mjjiwh37b0m3m1hijxifnchrw76i-libpciaccess-0.16.drv",["out"]) ,("/gnu/store/3fy3bf7wysi1n1qz9jz8xzx11sgy8m6d-git-2.41.0.drv",["out"]) ,("/gnu/store/3r0r8j76l0qvxasmb7rgn7lvpikjdyn1-libxdmcp-1.1.3.drv",["out"]) ,("/gnu/store/4jfy1ca1d5772z15jcyk1v8wdwdcllbi-gzip-1.12.drv",["out"]) ,("/gnu/store/5nvwagz2hphvlax2bnj93smr1rgrzr8l-libx11-1.8.1.drv",["out"]) ,("/gnu/store/64vwaah2spd7q66hji6sm1j2fl6pd1rn-diffutils-3.8.drv",["out"]) ,("/gnu/store/6k4xxkp725r09vkn7rz2gc50asjjhpkk-xorgproto-2022.2.drv",["out"]) ,("/gnu/store/91b6yraa6qax7lq7riqg1ag6lql2gfzi-tar-1.34.drv",["out"]) ,("/gnu/store/9kcv1x0lrf6fdck2j42zarxrvjzxxznv-coreutils-9.1.drv",["out"]) ,("/gnu/store/9piaq0aaf202r1gq7crig1cr131kx8zn-file-5.44.drv",["out"]) ,("/gnu/store/9y28bf3ywai2ybhr92c904s3cxsc8apx-libpthread-stubs-0.4.drv",["out"]) ,("/gnu/store/ak17xsjb4zcw7sf0r0lxxiy4xmh57i2h-findutils-4.9.0.drv",["out"])
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hello, Ulf Herrman skribis: > That and a growing thirst for a nuclear option for package rewriting > brought about by trying to debug deep transformations while > simultaneously experimenting with rewriting a manifest of around 270 > packages. On that topic of a catch-all option for rewriting: there’s an unused procedure called ‘map-derivation’, which performs rewriting at the level of derivations. It’s harder to use, more expensive, but who knows, perhaps we’ll find a motivating use case… (Allowing for graph rewriting has been a major goal for me since the beginning.) > There are really several distinct issues at play here: > 1. The case of #:qtbase and #:guile being invisible to package-mapping. >This is what I first noticed, and cannot be fixed without modifying >the build systems. This is what prompted looking for packages in >package and bag arguments, and recursing into lists therein (just in >case an argument took a list of packages). How are #:qtbase and #:guile invisible to package mapping? They appear in the bag inputs and thus are definitely visible to ‘package-mapping’, as a I showed with the CMake and Python examples in this thread. For good measure :-) here’s an example with #:qtbase: --8<---cut here---start->8--- $ ./pre-inst-env guix build qgit -n substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% 0.4 MB would be downloaded: /gnu/store/7b20q17yg90b62404chgbnwgvd6ry1qf-qgit-2.10 $ ./pre-inst-env guix build qgit -n --with-latest=qtbase following redirection to `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/'... following redirection to `https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/'... guix build: warning: cannot authenticate source of 'qtbase', version 6.6.0 Starting download of /tmp/guix-file.CTehnY >From >https://mirrors.ocf.berkeley.edu/qt/official_releases/qt/6.6/6.6.0/submodules/qtbase-everywhere-src-6.6.0.tar.xz... …-src-6.6.0.tar.xz 46.1MiB 12.9MiB/s 00:04 ▕██▏ 100.0% substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... 100.0% The following derivations would be built: /gnu/store/paixxkdaakv55bffggxx4l9hiknl8i5r-qgit-2.10.drv /gnu/store/f9fdjk1g1s1aqmlmi4clla2kqns7283v-qtbase-6.6.0.drv 0.4 MB would be downloaded: /gnu/store/nl9dadzfmjm9wg7v3r31jkx773dl683x-module-import-compiled /gnu/store/6zryxmypw0wygayc9pvhyxkx47w0lyci-gperf-3.1 /gnu/store/a57n7wy8mdi7l52pr4zg07132blgj5xp-qgit-2.10-checkout --8<---cut here---end--->8--- Now, package transformation options are almost all implemented in terms of ‘package-input-rewriting/spec’, and I have to admit that it’s much easier to use than ‘package-mapping’. The latter is low-level and it can be a source of headaches. > 2. The (perceived) case of packages hiding inside arguments. In >hindsight, this was probably actually (3) causing this, though it's >hard to tell because I discovered it last. I attempted to resolve >this by recursing through s and s. Yeah. I think we have to understand that “hiding” is to some extent inevitable; is more concise than , which is more concise than . There are extra “inputs” showing up when we go from one abstraction to the one below. > 3. `this-package' referring to the inherited package in thunked fields, >rather than the package inheriting them. This is what prompted the >use of package-{inputs,arguments,etc}-with-package. Ah yes, I agree; I reported it here: https://issues.guix.gnu.org/50335 I think we can discuss it separately though, in that bug report probably. > (1) could be resolved in several ways. I'm partial to looking for > arguments whose values are packages and transforming them (when #:deep? > #t is specified), and adjusting the build systems to make that work > consistently, which is what I've done. > > (2) is probably not an issue, though it occurs to me that the technique > of recursively searching through arguments looking for packages could be > used to implement a sort of automated "transformability" check, which > could help a lot when trying to debug transformations. OK. > (3) is a major issue; the entire strategy of using `this-package-input' > to enable transformations breaks because of it. My fix works for me at > least, though it requires exposing additional low-level procedures and > transformation authors using them. I'll open another bug about it, as > requested. Yeah, understood. Thanks, Ludo’.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hello, Maxim Cournoyer skribis: >> It’s indeed the case that arguments can capture references to packages >> that won’t be caught by ‘package-mapping’. For instance, if you write: >> >> (package >> … >> (arguments (list … #~(whatever #$coreutils >> >> … then ‘coreutils’ here cannot be replaced. >> >> To address this, the recommendation is to always add dependencies to >> input fields and to use self-references within arguments. The example >> above should be written like this: >> >> (package >> … >> (arguments >> (list … #~(whatever #$(this-package-input "coreutils") >> >> It’s just a recommendation and one can perfectly ignore it, and I >> suppose that was the impetus for this patch. >> >> This is one of the things discussed while designing this change: >> >> https://guix.gnu.org/en/blog/2021/the-big-change/ >> (search for “self-referential records”) >> >> https://issues.guix.gnu.org/49169 >> >> My take was and still is that it’s an acceptable limitation. Packagers >> need to follow the guideline above if they want proper support for >> rewriting, ‘guix graph’, and other tools. >> >> WDYT? > > But not all packages found in a bag come from inputs; they may be > provided as an argument to the build system (cmake, meson, qt, etc. all > allow for this). Yes, but these packages (#:cmake for ‘cmake-build-system’, #:python for ‘python-build-system’, etc.) become inputs of the bag, and are taken into account, as in this example: --8<---cut here---start->8--- $ ./pre-inst-env guix show tinyxml2 name: tinyxml2 version: 8.0.0 outputs: + out: everything systems: x86_64-linux i686-linux dependencies: location: gnu/packages/xml.scm:1295:2 homepage: http://www.grinninglizard.com/tinyxml2/ license: Zlib synopsis: Small XML parser for C++ description: TinyXML2 is a small and simple XML parsing library for the C++ programming + language. $ ./pre-inst-env guix build tinyxml2 The following graft will be made: /gnu/store/qf8w8ch1mrxk6k34g2d6bisny5pq8gnv-tinyxml2-8.0.0.drv applying 2 grafts for tinyxml2-8.0.0 ... grafting '/gnu/store/ndzyiwhypsbgrbaz7rmz3649b6ghlfal-tinyxml2-8.0.0' -> '/gnu/store/2da5frl36gd51653bjjypqvwdk0bh25v-tinyxml2-8.0.0'... successfully built /gnu/store/qf8w8ch1mrxk6k34g2d6bisny5pq8gnv-tinyxml2-8.0.0.drv /gnu/store/2da5frl36gd51653bjjypqvwdk0bh25v-tinyxml2-8.0.0 $ ./pre-inst-env guix build tinyxml2 --with-input=cmake-minimal=cmake The following derivation will be built: /gnu/store/hx0hx1nrmxzdhy9yw1d0md2q78y4spd4-tinyxml2-8.0.0.drv 8.4 MB will be downloaded: /gnu/store/qwy25ivr63087x9fdl9km0m44hn5bimg-cmake-3.25.1 /gnu/store/3vh9b2i93na8wwdrj6n0q2qbgd2fzxik-tinyxml2-8.0.0-checkout [...] /gnu/store/cmn551x8iksy44cdqypyq2129lm8ym60-tinyxml2-8.0.0 --8<---cut here---end--->8--- (In this case I had to apply the patch below because ‘cmake-minimal’ was mistakenly marked as hidden which, as implemented by eee95b5a879b7096dffd533f24107cf8926b621e, meant that it could not be replaced.) I gave a similar example with ‘python-build-system’ here: https://issues.guix.gnu.org/65665#10 My understanding is that understanding is that this patch series is about addressing cases as I explained above, where ‘arguments’ refer to packages directly instead of using self references. Do I get this right, Ulf? Thanks, Ludo’. diff --git a/gnu/packages/cmake.scm b/gnu/packages/cmake.scm index bc14286070..1592703f8d 100644 --- a/gnu/packages/cmake.scm +++ b/gnu/packages/cmake.scm @@ -263,6 +263,7 @@ (define-public cmake-minimal (package (inherit cmake-bootstrap) (name "cmake-minimal") +(properties (alist-delete 'hidden? (package-properties cmake-bootstrap))) (source (origin (inherit (package-source cmake-bootstrap)) ;; Purge CMakes bundled dependencies as they are no longer needed.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Ludovic Courtès writes: > Ulf Herrman skribis: > >> -(define (build-system-with-package-mapping bs rewrite) >> +(define (build-system-with-package-mapping bs rewrite-input >> rewrite-argument) >>"Return a variant of BS, a build system, that rewrites a bag's inputs by >> passing them through REWRITE, a procedure that takes an input tuplet and >> returns a \"rewritten\" input tuplet." >> @@ -1442,9 +1442,10 @@ (define (build-system-with-package-mapping bs rewrite) >> (let ((lowered (apply lower args))) >>(bag >> (inherit lowered) >> -(build-inputs (map rewrite (bag-build-inputs lowered))) >> -(host-inputs (map rewrite (bag-host-inputs lowered))) >> -(target-inputs (map rewrite (bag-target-inputs lowered)) >> +(build-inputs (map rewrite-input (bag-build-inputs lowered))) >> +(host-inputs (map rewrite-input (bag-host-inputs lowered))) >> +(target-inputs (map rewrite-input (bag-target-inputs lowered))) >> +(arguments (map rewrite-argument (bag-arguments lowered)) > > Aah, now I understand. :-) > > It’s indeed the case that arguments can capture references to packages > that won’t be caught by ‘package-mapping’. For instance, if you write: > > (package > … > (arguments (list … #~(whatever #$coreutils > > … then ‘coreutils’ here cannot be replaced. > > To address this, the recommendation is to always add dependencies to > input fields and to use self-references within arguments. The example > above should be written like this: > > (package > … > (arguments > (list … #~(whatever #$(this-package-input "coreutils") > > It’s just a recommendation and one can perfectly ignore it, and I > suppose that was the impetus for this patch. That and a growing thirst for a nuclear option for package rewriting brought about by trying to debug deep transformations while simultaneously experimenting with rewriting a manifest of around 270 packages. There are really several distinct issues at play here: 1. The case of #:qtbase and #:guile being invisible to package-mapping. This is what I first noticed, and cannot be fixed without modifying the build systems. This is what prompted looking for packages in package and bag arguments, and recursing into lists therein (just in case an argument took a list of packages). 2. The (perceived) case of packages hiding inside arguments. In hindsight, this was probably actually (3) causing this, though it's hard to tell because I discovered it last. I attempted to resolve this by recursing through s and s. 3. `this-package' referring to the inherited package in thunked fields, rather than the package inheriting them. This is what prompted the use of package-{inputs,arguments,etc}-with-package. (1) could be resolved in several ways. I'm partial to looking for arguments whose values are packages and transforming them (when #:deep? #t is specified), and adjusting the build systems to make that work consistently, which is what I've done. (2) is probably not an issue, though it occurs to me that the technique of recursively searching through arguments looking for packages could be used to implement a sort of automated "transformability" check, which could help a lot when trying to debug transformations. (3) is a major issue; the entire strategy of using `this-package-input' to enable transformations breaks because of it. My fix works for me at least, though it requires exposing additional low-level procedures and transformation authors using them. I'll open another bug about it, as requested. > This is one of the things discussed while designing this change: > > https://guix.gnu.org/en/blog/2021/the-big-change/ > (search for “self-referential records”) > > https://issues.guix.gnu.org/49169 > > My take was and still is that it’s an acceptable limitation. Packagers > need to follow the guideline above if they want proper support for > rewriting, ‘guix graph’, and other tools. > > WDYT? I think it's probably reasonable, though I would like it if there were tooling in place to detect cases where said guidelines are not followed, so as to aid in debugging and verifying that a transformation worked as intended. - Ulf signature.asc Description: PGP signature
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Maxim Cournoyer writes: > Any build system accepting package as arguments are subject to this > problem, if I understand correctly. It's not quite everywhere a package is accepted as an argument: there are many cases where a package is passed as a package argument but doesn't end up in the arguments list of the corresponding bag. This is usually the case because that argument is only overriding a default package that is just added as an input to the bag, with no special treatment aside from being an implicit input. For example, #:cmake in cmake-build-system works this way. This is however not the case for #:qtbase in qt-build-system, and, much more prominently, for #:guile. Neither qtbase nor guile are added to bag inputs implicitly, but they do end up in the final builder, even if not specified. Concretely, this looks like this: - (use-modules (guix packages) (guix profiles) (gnu packages base)) (define guile-named-lyle (package (inherit (default-guile)) (name "lyle"))) ;; contrived example that only replaces hello's immediate dependencies (define hello-transformer (package-mapping (lambda (p0) (if (eq? p0 (default-guile)) guile-named-lyle p0)) (lambda (p) (not (eq? p hello))) #:deep? #t)) (define hello-with-lyle (hello-transformer hello)) (packages->manifest (list hello hello-with-lyle)) ;; $ guix build --derivations --manifest=THISFILE ;; Expectation: build for hello-with-lyle is done with guile-named-lyle. ;; Reality: derivation for hello-with-lyle is the same as hello. - (and I have confirmed that the above results in guile-named-lyle being used with the proposed patches applied) A similar problem manifests when one tries replacing (default-qtbase) in a package using qt-build-system. Both it and guile are in bag arguments and not inputs because it's not enough that they be included as inputs, the builder must also know which one is considered "used by the build system" if there are multiple. One could argue that this ambiguity also exists with other build systems - "which cmake should be used, which gcc should be used", etc - but they choose to resolve it with "eh, whatever shows up first in PATH matching the required name is probably fine". It's not unimaginable that there could be cases where explicitly specifying which package should be used for a particular role would be necessary, though. - Ulf signature.asc Description: PGP signature
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi, Ludovic Courtès writes: > Ulf Herrman skribis: > >> -(define (build-system-with-package-mapping bs rewrite) >> +(define (build-system-with-package-mapping bs rewrite-input >> rewrite-argument) >>"Return a variant of BS, a build system, that rewrites a bag's inputs by >> passing them through REWRITE, a procedure that takes an input tuplet and >> returns a \"rewritten\" input tuplet." >> @@ -1442,9 +1442,10 @@ (define (build-system-with-package-mapping bs rewrite) >> (let ((lowered (apply lower args))) >>(bag >> (inherit lowered) >> -(build-inputs (map rewrite (bag-build-inputs lowered))) >> -(host-inputs (map rewrite (bag-host-inputs lowered))) >> -(target-inputs (map rewrite (bag-target-inputs lowered)) >> +(build-inputs (map rewrite-input (bag-build-inputs lowered))) >> +(host-inputs (map rewrite-input (bag-host-inputs lowered))) >> +(target-inputs (map rewrite-input (bag-target-inputs lowered))) >> +(arguments (map rewrite-argument (bag-arguments lowered)) > > Aah, now I understand. :-) > > It’s indeed the case that arguments can capture references to packages > that won’t be caught by ‘package-mapping’. For instance, if you write: > > (package > … > (arguments (list … #~(whatever #$coreutils > > … then ‘coreutils’ here cannot be replaced. > > To address this, the recommendation is to always add dependencies to > input fields and to use self-references within arguments. The example > above should be written like this: > > (package > … > (arguments > (list … #~(whatever #$(this-package-input "coreutils") > > It’s just a recommendation and one can perfectly ignore it, and I > suppose that was the impetus for this patch. > > This is one of the things discussed while designing this change: > > https://guix.gnu.org/en/blog/2021/the-big-change/ > (search for “self-referential records”) > > https://issues.guix.gnu.org/49169 > > My take was and still is that it’s an acceptable limitation. Packagers > need to follow the guideline above if they want proper support for > rewriting, ‘guix graph’, and other tools. > > WDYT? But not all packages found in a bag come from inputs; they may be provided as an argument to the build system (cmake, meson, qt, etc. all allow for this). -- Thanks, Maxim
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi Ludovic, Ludovic Courtès writes: > Hi Ulf, > > Ulf Herrman skribis: > >> In short, the current approach of having the build-system lower >> procedure leave the arguments mostly unchanged and letting the >> bag->derivation procedure (the "bag builder") fill in lots of defaults >> means that there are implicit inputs that cannot be touched at the >> package level without adding special logic for every single build system >> that does something like this. > > Could you share an example of what is fixed by these changes? Ulf had mentioned the problems they were facing in the original post of this issue: > The problem with this approach is that it doesn't affect the bag > arguments. This means that packages passed in as arguments may still > end up being used without being transformed. Worse still, packages > *not* passed in as arguments may end up being used *unless one is > explicitly passed in as an argument*, as is the case with > qt-build-system's #:qtbase argument. Examples would be an Qt packages, or packages using #:meson meson-variant. Any build system accepting package as arguments are subject to this problem, if I understand correctly. I hope that helps, -- Thanks, Maxim
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Ulf Herrman skribis: > -(define (build-system-with-package-mapping bs rewrite) > +(define (build-system-with-package-mapping bs rewrite-input rewrite-argument) >"Return a variant of BS, a build system, that rewrites a bag's inputs by > passing them through REWRITE, a procedure that takes an input tuplet and > returns a \"rewritten\" input tuplet." > @@ -1442,9 +1442,10 @@ (define (build-system-with-package-mapping bs rewrite) > (let ((lowered (apply lower args))) >(bag > (inherit lowered) > -(build-inputs (map rewrite (bag-build-inputs lowered))) > -(host-inputs (map rewrite (bag-host-inputs lowered))) > -(target-inputs (map rewrite (bag-target-inputs lowered)) > +(build-inputs (map rewrite-input (bag-build-inputs lowered))) > +(host-inputs (map rewrite-input (bag-host-inputs lowered))) > +(target-inputs (map rewrite-input (bag-target-inputs lowered))) > +(arguments (map rewrite-argument (bag-arguments lowered)) Aah, now I understand. :-) It’s indeed the case that arguments can capture references to packages that won’t be caught by ‘package-mapping’. For instance, if you write: (package … (arguments (list … #~(whatever #$coreutils … then ‘coreutils’ here cannot be replaced. To address this, the recommendation is to always add dependencies to input fields and to use self-references within arguments. The example above should be written like this: (package … (arguments (list … #~(whatever #$(this-package-input "coreutils") It’s just a recommendation and one can perfectly ignore it, and I suppose that was the impetus for this patch. This is one of the things discussed while designing this change: https://guix.gnu.org/en/blog/2021/the-big-change/ (search for “self-referential records”) https://issues.guix.gnu.org/49169 My take was and still is that it’s an acceptable limitation. Packagers need to follow the guideline above if they want proper support for rewriting, ‘guix graph’, and other tools. WDYT? Thanks, Ludo’.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi Ulf, Ulf Herrman skribis: > In short, the current approach of having the build-system lower > procedure leave the arguments mostly unchanged and letting the > bag->derivation procedure (the "bag builder") fill in lots of defaults > means that there are implicit inputs that cannot be touched at the > package level without adding special logic for every single build system > that does something like this. Could you share an example of what is fixed by these changes? Let’s take ‘python-itsdangerous’ as an example. It has zero inputs, only implicit dependencies, yet I can do this: --8<---cut here---start->8--- $ guix build python-itsdangerous -n substitute: updating substitutes from 'http://192.168.1.48:8123'... 100.0% 0.0 MB would be downloaded: /gnu/store/yhvhrmqd5znjs7vsq8kc55nc3rkg4w6x-python-itsdangerous-2.0.1 $ guix build python-itsdangerous --with-input=python=python2 -n The following derivations would be built: /gnu/store/b1hji2qzdiwfg2wx11l1fyjrgiy0f50v-python-itsdangerous-2.0.1.drv /gnu/store/5gibs6x75acc6j0g0rh8m66191l9wq12-python-wrapper-3.10.7.drv 0.1 MB would be downloaded: /gnu/store/b2qv97jbih850zn35b2j84n2acj079cv-itsdangerous-2.0.1.tar.gz $ guix gc --references /gnu/store/5gibs6x75acc6j0g0rh8m66191l9wq12-python-wrapper-3.10.7.drv |grep python /gnu/store/n0snl506x5bbs5c2496blh79yil3pf44-python2-2.7.18.drv /gnu/store/whwrah24q7syyiqra16sm9mjvdxld1pv-python-wrapper-3.10.7-builder --8<---cut here---end--->8--- ‘python’ above is an implicit dependency, yet it was suitably replaced by ‘--with-input’. In what cases does ‘package-mapping #:deep? #t’ miss implicit inputs? Thanks, Ludo’.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi, Ulf Herrman skribis: > I've also been using this patch, which rebinds `this-package' within > package variants to refer instead to the variant rather than the > original. Could you send it as a separate issue? (I think I once reported a bug for this issue, but I can’t find it.) Ludo’.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hi, On Thu, 31 Aug 2023 at 15:14, Ulf Herrman wrote: > I propose that we have the build system lower procedure (that is, the > one that converts from package to bag) completely fill in the argument > list with all defaults, and we modify build-system-with-package-mapping > to transform all arguments that look like a package or a list of > packages (or maybe even a tree containing packages). On principle, this looks a good idea. Do you already have an implementation? Because transformations lead to headache and sometimes the implementation is hard, well evil, details and all that. :-) Cheers, simon
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Hello Ulf, Ulf Herrman writes: > #:deep? #t currently works by interposing a dummy build system that > lowers the package to a bag using the original build system, then > applies the supplied transformation to all of the bag's inputs, then > returns a new bag with the new inputs. > > The problem with this approach is that it doesn't affect the bag > arguments. This means that packages passed in as arguments may still > end up being used without being transformed. Worse still, packages > *not* passed in as arguments may end up being used *unless one is > explicitly passed in as an argument*, as is the case with > qt-build-system's #:qtbase argument. > > In short, the current approach of having the build-system lower > procedure leave the arguments mostly unchanged and letting the > bag->derivation procedure (the "bag builder") fill in lots of defaults > means that there are implicit inputs that cannot be touched at the > package level without adding special logic for every single build system > that does something like this. This is indeed sub-optimal! > I propose that we have the build system lower procedure (that is, the > one that converts from package to bag) completely fill in the argument > list with all defaults, and we modify build-system-with-package-mapping > to transform all arguments that look like a package or a list of > packages (or maybe even a tree containing packages). [...] > What do you think? Like Csepp, I like your proposition! I'm CC'ing the core team, in case they have something to add to it, but otherwise, we only need a volunteer to turn it into code :-). -- Thanks, Maxim
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
Ulf Herrman writes: > [[PGP Signed Part:Undecided]] > #:deep? #t currently works by interposing a dummy build system that > lowers the package to a bag using the original build system, then > applies the supplied transformation to all of the bag's inputs, then > returns a new bag with the new inputs. > > The problem with this approach is that it doesn't affect the bag > arguments. This means that packages passed in as arguments may still > end up being used without being transformed. Worse still, packages > *not* passed in as arguments may end up being used *unless one is > explicitly passed in as an argument*, as is the case with > qt-build-system's #:qtbase argument. > > In short, the current approach of having the build-system lower > procedure leave the arguments mostly unchanged and letting the > bag->derivation procedure (the "bag builder") fill in lots of defaults > means that there are implicit inputs that cannot be touched at the > package level without adding special logic for every single build system > that does something like this. > > I propose that we have the build system lower procedure (that is, the > one that converts from package to bag) completely fill in the argument > list with all defaults, and we modify build-system-with-package-mapping > to transform all arguments that look like a package or a list of > packages (or maybe even a tree containing packages). > > Many large-scale package transformations have their purpose defeated if > even a single package slips through and has to be built or downloaded > separately (e.g. "I wanted to use a more minimal version of P, and now I > have both the original version of P *and* a more minimal version of P > *and* a duplicate copy of a large portion of the package graph..."). In > fact, in some situations, this could cause exponential growth of the > number of packages, e.g. a transformation is applied to qtbase and its > dependencies, but not to the implicit version and its dependencies, so > now all the dependencies of qtbase are duplicated, and if this happens > again at a higher level with something that depends on a package with > qt-build-system, now there are four duplicate subgraphs, and so on. > > What do you think? > > - Ulf > > [[End of PGP Signed Part]] Sounds like a good idea to me.
bug#65665: package-mapping with #:deep? #t doesn't get all the implicit inputs
#:deep? #t currently works by interposing a dummy build system that lowers the package to a bag using the original build system, then applies the supplied transformation to all of the bag's inputs, then returns a new bag with the new inputs. The problem with this approach is that it doesn't affect the bag arguments. This means that packages passed in as arguments may still end up being used without being transformed. Worse still, packages *not* passed in as arguments may end up being used *unless one is explicitly passed in as an argument*, as is the case with qt-build-system's #:qtbase argument. In short, the current approach of having the build-system lower procedure leave the arguments mostly unchanged and letting the bag->derivation procedure (the "bag builder") fill in lots of defaults means that there are implicit inputs that cannot be touched at the package level without adding special logic for every single build system that does something like this. I propose that we have the build system lower procedure (that is, the one that converts from package to bag) completely fill in the argument list with all defaults, and we modify build-system-with-package-mapping to transform all arguments that look like a package or a list of packages (or maybe even a tree containing packages). Many large-scale package transformations have their purpose defeated if even a single package slips through and has to be built or downloaded separately (e.g. "I wanted to use a more minimal version of P, and now I have both the original version of P *and* a more minimal version of P *and* a duplicate copy of a large portion of the package graph..."). In fact, in some situations, this could cause exponential growth of the number of packages, e.g. a transformation is applied to qtbase and its dependencies, but not to the implicit version and its dependencies, so now all the dependencies of qtbase are duplicated, and if this happens again at a higher level with something that depends on a package with qt-build-system, now there are four duplicate subgraphs, and so on. What do you think? - Ulf signature.asc Description: PGP signature