Package: guix-patches;
Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Date: Fri, 3 Feb 2023 16:20:02 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Christopher Baines <mail <at> cbaines.net>, Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>, 61255 <at> debbugs.gnu.org Subject: [bug#61255] (%guile-for-build) default in ‘computed-file’ Date: Thu, 23 Feb 2023 21:38:22 -0500
Hi Ludovic,
Ludovic Courtès <ludo <at> gnu.org> writes:
> Hello!
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> Hi Ludovic!
>>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>
>>> Hi Maxim,
>>>
>>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>>
>>>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>>>
>>>>> Hello!
>>>>>
>>>>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>>>>
>>>>>> * guix/gexp.scm (computed-file): Set the default value of the #:guile argument
>>>>>> to that of the %guile-for-build parameter.
>>>>>
>>>>> [...]
>>>>>
>>>>>> (define* (computed-file name gexp
>>>>>> - #:key guile (local-build? #t) (options '()))
>>>>>> + #:key (guile (%guile-for-build))
>>>>>> + (local-build? #t) (options '()))
>>>>>
>>>>> I think that would lead ‘computed-file’ to pick (%guile-for-build) at
>>>>> the wrong time (time of call instead of time of lowering).
>>>>>
>>>>> Commit ab25eb7caaf5571cc9f8d6397a1eae127d7e29d1 made it #f such that
>>>>> ‘gexp->derivation’ gets to resolve it at the “right” time.
>>>>
>>>> I see! I think you are right. Would making the change in the
>>>> associated gexp compiler do the right thing? Currently it ignores the
>>>> %guile-for-build fluid as set in the tests/pack.scm test suite for
>>>> example. Something like this:
>>>
>>> I don’t fully understand the context. My preference would go to doing
>>> like the ‘computed-file’ tests in ‘tests/gexp.scm’, where we explicitly
>>> pass #:guile %bootstrap-guile.
>>
>> With the refactoring done in patch 3/5 ("pack: Extract
>> populate-profile-root from self-contained-tarball/builder."), a
>> computed-file is used in the factorized building block
>> 'populate-profile-root'. Without this patch, the tests making use of it
>> would attempt to build Guile & friends in the test store.
>>
>>> That said, it seems like patch #5 in this series doesn’t actually use
>>> ‘computed-file’ in ‘tests/pack.scm’, does it?
>>
>> It does, indirectly.
>>
>> I hope that helps!
>
> I’m really not sure what the impact of
> 68775338a510f84e63657ab09242d79e726fa457 is, nor whether it was the only
> solution to the problem.
>
> One thing that probably happens is that (default-guile) is now never
> used for <computed-file>, contrary to what was happening before. The
> spirit is that (default-guile) would be used as the default for all the
> declarative file-like objects; gexp compilers refer to (default-guile),
> not (%guile-for-build).
>
> Importantly, (%guile-for-build) is a derivation, possibly built for
> another system, whereas (default-guile) is a package, which allows
> ‘lower-object’ to return the derivation for the right system type.
I assumed the purpose of the %guile-for-build fluid was to override the
value of the guile used in some conditions, such as during tests
(e.g. the '(set-guile-for-build (default-guile))' calls inside the store
monad in tests/pack.scm). It's honored for gexp->derivation, but isn't
honored for computed-file, which is supposed to be its declarative
counterpart. This problem was only exposed when factoring out
'populate-profile-root' as a computed-file object in
68380db4c40a2ee1156349a87254fd7b1f1a52d5 ("pack: Extract
populate-profile-root from self-contained-tarball/builder.")
> Overall, I think this change should be reverted but of course, we should
> find a solution to the problem you hit in the first place.
>
> I hope this makes sense to you.
See the problem it solves below. If we revert this now, we'd have to
mark the 'self-contained-tarball' as an expected fail until we find a a
better solution.
The problem it solves is this: after reverting the change with:
> modified guix/gexp.scm
> @@ -601,7 +601,7 @@ (define-gexp-compiler (computed-file-compiler (file <computed-file>)
> ;; gexp.
> (match file
> (($ <computed-file> name gexp guile options)
> - (mlet %store-monad ((guile (lower-object (or guile (%guile-for-build)
> + (mlet %store-monad ((guile (lower-object (or guile ;(%guile-for-build)
> (default-guile))
> system #:target #f)))
> (apply gexp->derivation name gexp #:guile-for-build guile
Running the pack.scm tests:
$ make check TESTS=tests/pack.scm
Fails with a timeout, because the %guile-for-build is not honored by a
computed-file derivation, and it goes on building the non-bootstrap
build-side guile, gcc, etc. in the test store (see: pack.log):
--8<---------------cut here---------------start------------->8---
gcc-10.3.0/gcc/targhooks.h
gcc-10.3.0/gcc/testsuite/
gcc-10.3.0/gcc/testsuite/.gitattributes
gcc-10.3.0/gcc/testsuite/ChangeLog
gcc-10.3.0/gcc/testsuite/ChangeLog-1993-2007
gcc-10.3.0/gcc/testsuite/ChangeLog-2008
gcc-10.3.0/gcc/testsuite/ChangeLog-2009
gcc-10.3.0/gcc/testsuite/ChangeLog-2010
gcc-10.3.0/gcc/testsuite/ChangeLog-2011
gcc-10.3.0/gcc/testsuite/ChangeLog-2012
gcc-10.3.0/gcc/testsuite/ChangeLog-2013
gcc-10.3.0/gcc/testsuite/ChangeLog-2014
gcc-10.3.0/gcc/testsuite/ChangeLog-2015
gcc-10.3.0/gcc/testsuite/ChangeLog-2016
gcc-10.3.0/gcc/testsuite/ChangeLog-2017
gcc-10.3.0/gcc/testsuite/ChangeLog-2018
building of `/home/maxim/src/guix/test-tmp/store/hp86j4850ajphhs1hyryis5nj93pv66l-gcc-10.3.0.tar.xz.drv' timed out after 300 seconds
@ build-failed /home/maxim/src/guix/test-tmp/store/hp86j4850ajphhs1hyryis5nj93pv66l-gcc-10.3.0.tar.xz.drv - timeout
killing process 4149
cannot build derivation `/home/maxim/src/guix/test-tmp/store/82yb9zwxdwhmacz36pjrrzzmgjgakavy-gcc-10.3.0.drv': 1 dependencies couldn't be built
@ build-started /home/maxim/src/guix/test-tmp/store/8dfjl4594zgb7wi3icw8s9z3rr3pck6x-gcc-4.9.4.tar.xz.drv - x86_64-linux /home/maxim/src/guix/test-tmp/var/log/guix/drvs/8d//fjl4594zgb7wi3icw8s9z3rr3pck6x-gcc-4.9.4.tar.xz.drv.gz 4611
cannot build derivation `/home/maxim/src/guix/test-tmp/store/hcv6vh1gx5fkw62l3nravi1aqhi8cq60-gcc-cross-boot0-10.3.0.drv': 1 dependencies couldn't be built
killing process 4611
cannot build derivation `/home/maxim/src/guix/test-tmp/store/1ihb1yadv4dfbqhfcgn1cyvsl8444yaw-guile-3.0.7.drv': 1 dependencies couldn't be built
cannot build derivation `/home/maxim/src/guix/test-tmp/store/6g7fhyr1b84b5qg8nwn46hkrg55i8c2q-profile-directory.drv': 1 dependencies couldn't be built
cannot build derivation `/home/maxim/src/guix/test-tmp/store/apm8bjvzs1n707lagw0spzr2m2nc0p4v-pack.tar.gz.drv': 1 dependencies couldn't be built
cannot build derivation `/home/maxim/src/guix/test-tmp/store/syiq7lmx3v0pkrjp5wqd5kfapqpxpki3-check-tarball.drv': 1 dependencies couldn't be built
test-name: self-contained-tarball
location: /home/maxim/src/guix/tests/pack.scm:80
source:
+ (test-assert
+ "self-contained-tarball"
+ (let ((guile (package-derivation %store %bootstrap-guile)))
+ (run-with-store
+ %store
+ (mlet* %store-monad
+ ((profile
+ ->
+ (profile
+ (content
+ (packages->manifest (list %bootstrap-guile)))
+ (hooks '())
+ (locales? #f)))
+ (tarball
+ (self-contained-tarball
+ "pack"
+ profile
+ #:symlinks
+ '(("/bin/Guile" -> "bin/guile"))
+ #:compressor
+ %gzip-compressor
+ #:archiver
+ %tar-bootstrap))
+ (check (gexp->derivation
+ "check-tarball"
+ (with-imported-modules
+ '((guix build utils))
+ (gexp (begin
+ (use-modules
+ (guix build utils)
+ (srfi srfi-1))
+ (define store
+ (string-append
+ "."
+ (%store-directory)
+ "/"))
+ (define (canonical? file)
+ (let ((st (lstat file)))
+ (or (not (string-prefix? store file))
+ (eq? 'symlink (stat:type st))
+ (and (= 1 (stat:mtime st))
+ (zero? (logand
+ 146
+ (stat:mode st)))))))
+ (define bin
+ (string-append
+ "."
+ (ungexp profile)
+ "/bin"))
+ (setenv
+ "PATH"
+ (string-append
+ (ungexp %tar-bootstrap)
+ "/bin"))
+ (system* "tar" "xvf" (ungexp tarball))
+ (mkdir (ungexp output))
+ (exit (and (file-exists?
+ (string-append bin "/guile"))
+ (file-exists? store)
+ (every canonical?
+ (find-files
+ "."
+ (const #t)
+ #:directories?
+ #t))
+ (string=?
+ (string-append
+ (ungexp %bootstrap-guile)
+ "/bin")
+ (readlink bin))
+ (string=?
+ (string-append
+ ".."
+ (ungexp profile)
+ "/bin/guile")
+ (readlink "bin/Guile"))))))))))
+ (built-derivations (list check)))
+ #:guile-for-build
+ guile)))
actual-value: #f
actual-error:
+ (%exception
+ #<&store-protocol-error message: "build of `/home/maxim/src/guix/test-tmp/store/syiq7lmx3v0pkrjp5wqd5kfapqpxpki3-check-tarball.drv' failed" status: 101>)
result: FAIL
--8<---------------cut here---------------end--------------->8---
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.