gnu/ci.go: Embeds build path, breaking reproducible builds

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Mathieu Othacehe
  • Vagrant Cascadian
Owner
unassigned
Submitted by
Vagrant Cascadian
Severity
normal
V
V
Vagrant Cascadian wrote on 24 Nov 2020 05:19
(address . bug-guix@gnu.org)
87r1ojgzdh.fsf@yucca
Something in gnu/ci.scm is embedding the build path when compiled into
gnu/ci.go, as can be seen:


./usr/lib/x86_64-linux-gnu/guile/3.0/site-ccache/gnu/ci.go
strings --all --bytes=8 {}
Offset 85, 15 lines modified Offset 85, 15 lines modified
...
92 /build/1st/guix-1.2.0~rc2/gnu/ci.scm 92 /build/2/guix-1.2.0~rc2/2nd/gnu/ci.scm

While guix builds of guix are typically built with a consistent build
path, it would be nice to fix this issue for other environments where
the build path may vary between builds.


My *wild* guess is it maybe has something to do with the use of
canonicalize-path:

(define (find-current-checkout arguments)
"Find the first checkout of ARGUMENTS that provided the current file.
Return #f if no such checkout is found."
(let ((current-root
(canonicalize-path
(string-append (dirname (current-filename)) "/.."))))
(find (lambda (argument)
(and=> (assq-ref argument 'file-name)
(lambda (name)
(string=? name current-root)))) arguments)))

Either directly or indirectly... does canonicalize-path resolve at build
time? run time? somewhere in-between? Or is this a red herring, and
something else entirely is responsible? I'll let a competent schemer
ponder this! :)


Thanks for all your help!


live well,
vagrant
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCX7yJ6gAKCRDcUY/If5cW
qlmmAQD+b4gNNqA6atRtAHhIqm5o4r5EqMgH/9uiGy4s5VsBpAD8CtYB6YkOAAsA
K5sxn7qvHLyvpGepXa2VrrmtWHNTUgg=
=zJG9
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 26 Nov 2020 22:39
(name . Vagrant Cascadian)(address . vagrant@reproducible-builds.org)
87blfj23yb.fsf@gnu.org
Hi!

Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:

Toggle quote (14 lines)
> My *wild* guess is it maybe has something to do with the use of
> canonicalize-path:
>
> (define (find-current-checkout arguments)
> "Find the first checkout of ARGUMENTS that provided the current file.
> Return #f if no such checkout is found."
> (let ((current-root
> (canonicalize-path
> (string-append (dirname (current-filename)) "/.."))))
> (find (lambda (argument)
> (and=> (assq-ref argument 'file-name)
> (lambda (name)
> (string=? name current-root)))) arguments)))

‘canonicalize-path’ is called at run time, so that’s fine. However,
‘current-filename’ is a macro that captures the source file name at
build time, so it’s the likely culprit here.

I was going to go with something like:
Toggle diff (14 lines)
diff --git a/gnu/ci.scm b/gnu/ci.scm
index 5548d9560e..0bacfbe025 100644
--- a/gnu/ci.scm
+++ b/gnu/ci.scm
@@ -488,7 +488,8 @@ valid."
Return #f if no such checkout is found."
(let ((current-root
(canonicalize-path
- (string-append (dirname (current-filename)) "/.."))))
+ (string-append (dirname (search-path %load-path "gnu/ci.scm"))
+ "/.."))))
(find (lambda (argument)
(and=> (assq-ref argument 'file-name)
(lambda (name)
… but I don’t think we can assume that the checkout is in the
‘%load-path’ when this code is executed. WDYT, Mathieu?

Looking at f71b0a0012d46bd30ead1a14ed58fd59647415e2, which introduced
this, there might be other options too.

Thanks,
Ludo’.
V
V
Vagrant Cascadian wrote on 27 Nov 2020 21:25
(name . Ludovic Courtès)(address . ludo@gnu.org)
878samh7j2.fsf@yucca
On 2020-11-26, Ludovic Courtès wrote:
Toggle quote (42 lines)
> Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:
>> My *wild* guess is it maybe has something to do with the use of
>> canonicalize-path:
>>
>> (define (find-current-checkout arguments)
>> "Find the first checkout of ARGUMENTS that provided the current file.
>> Return #f if no such checkout is found."
>> (let ((current-root
>> (canonicalize-path
>> (string-append (dirname (current-filename)) "/.."))))
>> (find (lambda (argument)
>> (and=> (assq-ref argument 'file-name)
>> (lambda (name)
>> (string=? name current-root)))) arguments)))
>
> ‘canonicalize-path’ is called at run time, so that’s fine. However,
> ‘current-filename’ is a macro that captures the source file name at
> build time, so it’s the likely culprit here.
>
> I was going to go with something like:
>
> diff --git a/gnu/ci.scm b/gnu/ci.scm
> index 5548d9560e..0bacfbe025 100644
> --- a/gnu/ci.scm
> +++ b/gnu/ci.scm
> @@ -488,7 +488,8 @@ valid."
> Return #f if no such checkout is found."
> (let ((current-root
> (canonicalize-path
> - (string-append (dirname (current-filename)) "/.."))))
> + (string-append (dirname (search-path %load-path "gnu/ci.scm"))
> + "/.."))))
> (find (lambda (argument)
> (and=> (assq-ref argument 'file-name)
> (lambda (name)
>
> … but I don’t think we can assume that the checkout is in the
> ‘%load-path’ when this code is executed. WDYT, Mathieu?
>
> Looking at f71b0a0012d46bd30ead1a14ed58fd59647415e2, which introduced
> this, there might be other options too.

This does resolve the reproducibility issues after some "quick" testing;
no idea if it breaks anything, though I didn't see any new test suite
failures (though my builds are patched to skip many tests)...


live well,
vagrant
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCX8FgoQAKCRDcUY/If5cW
qrW3AP4zrVVyDNDv2kxfNWWCrqfxOnmgYNKhXG8nlg0H9vb6pgEAttq02lrkEthU
RDqtXmcO+pjoMZmboFK6T3b4397S+go=
=G6Px
-----END PGP SIGNATURE-----

M
M
Mathieu Othacehe wrote on 1 Dec 2020 10:41
(name . Ludovic Courtès)(address . ludo@gnu.org)
87wny13lub.fsf@gnu.org
Hey,

Toggle quote (3 lines)
> … but I don’t think we can assume that the checkout is in the
> ‘%load-path’ when this code is executed. WDYT, Mathieu?

Cuirass happens to add checkouts to the %load-path just before loading
this file.

I tested your path, it works fine. I think it is acceptable as this (gnu
ci) interface needs a big rework anyway.

I can apply your patch if it's ok for you.

Thanks,

Mathieu
L
L
Ludovic Courtès wrote on 3 Dec 2020 14:23
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
87o8jbdnvb.fsf@gnu.org
Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

Toggle quote (6 lines)
>> … but I don’t think we can assume that the checkout is in the
>> ‘%load-path’ when this code is executed. WDYT, Mathieu?
>
> Cuirass happens to add checkouts to the %load-path just before loading
> this file.

Is that systematic? Isn’t it only when ‘load_path_inputs’ is non-empty?

Toggle quote (5 lines)
> I tested your path, it works fine. I think it is acceptable as this (gnu
> ci) interface needs a big rework anyway.
>
> I can apply your patch if it's ok for you.

Sure if you’re confident you can go ahead. :-)

Thanks,
Ludo’.
V
V
Vagrant Cascadian wrote on 7 Mar 22:46 +0100
(address . 44835-done@debbugs.gnu.org)
875xxxogva.fsf@wireframe
On 2020-12-03, Ludovic Courtès wrote:
Toggle quote (17 lines)
> Mathieu Othacehe <othacehe@gnu.org> skribis:
>
>>> … but I don’t think we can assume that the checkout is in the
>>> ‘%load-path’ when this code is executed. WDYT, Mathieu?
>>
>> Cuirass happens to add checkouts to the %load-path just before loading
>> this file.
>
> Is that systematic? Isn’t it only when ‘load_path_inputs’ is non-empty?
>
>> I tested your path, it works fine. I think it is acceptable as this (gnu
>> ci) interface needs a big rework anyway.
>>
>> I can apply your patch if it's ok for you.
>
> Sure if you’re confident you can go ahead. :-)

This looks to have been fixed some time ago in:

76bea3f8bcd951ded88dfb7f8cad5bc3e5a1701f ci: Remove hydra support.

live well,
vagrant
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCZeo1mQAKCRDcUY/If5cW
qgC/AP45REunfhTWgLsx/uj/E+L/tpY/RWNKAUhFlDmjwYzd4gD/efPNYzm8Rndc
c654SE2YJqWQwa/F/pS4KM+wfRHZVQs=
=6HvJ
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 44835
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