Guix Inferiors: Curious incorrect derivation output bug

  • Done
  • quality assurance status badge
Details
2 participants
  • Carl Dong
  • Ludovic Courtès
Owner
unassigned
Submitted by
Carl Dong
Severity
important
C
C
Carl Dong wrote on 24 Jul 2019 03:09
(name . bug-guix@gnu.org)(address . bug-guix@gnu.org)
TifHDa-L4paOnZQruSSv3YhpIdqMvwlk7pwnElq9azsMZwEdFv3-0usXgSQkV4nIOuKoJzv-BwIurwdecWYX-BTGUn2xZXAqQzefQp85W7M=@carldong.me
Hi all,

I've been working on improving the Guix build support on Bitcoin Core so that
it'll be ready to use for official releases.

One of the things that I wanted to do was to use a combination of channels and
inferiors to have reproducible builds across time. I have most of it set up, but
am running into some trouble. Here are the details:

I have a Guix channel set up at https://github.com/dongcarl/bitcoin-guix,where
I pin the Guix version to 6869b6635afd93872b8f0d9f2db0db4c0d765a86 in the
.guix-channel, and declare all my packages in packages.scm.

I have my manifest.scm here:
and it references the aforementioned Guix channel.

What I expect to happen is that now when I change my default profile's Guix
version by 'guix pull'ing, it won't affect the environment that's generated by
the manifest.scm. I've tested this with differing versions of Guix as my default
profile, and this seems to work with 7304d5623ab5cc35289cb1535cbd0d8a37691fac
and 7f1c69f5d32bee6b8b6b902a9ce445e04aa9d07d being my default profile. However,
I tried an older version b6dc08393e6a8313b88ce422fc3c1e4e9c0efc6f, and got the
following error:

```
guix environment: error: derivation `/gnu/store/r641vpqc9rfjhljf7rzfzwmkmpa642ls-info-dir.drv' has incorrect output `/gnu/store/q9hkdidycz3wq28xxgjq47bzx5s39k52-info-dir', should be `/gnu/store/z5hh2nl0h58b9f6hdxfwm00gjyxfcc3n-info-dir'
```

I remember my previous attempts at integrating channels and inferiors into our
workflow yielded a similar result, so I would like to know if this is a bug, and
how I might go about fixing this so that I can feel comfortable using inferiors
for our builds.

Nevertheless, thank you all for your hard work. The fact that we have inferiors
at all is marvelous!

Cheers,
Carl Dong
contact@carldong.me
"I fight for the users"
L
L
Ludovic Courtès wrote on 26 Jul 2019 09:50
(name . Carl Dong)(address . contact@carldong.me)(address . 36777@debbugs.gnu.org)
875znps08a.fsf@gnu.org
Hello Carl,

Carl Dong <contact@carldong.me> skribis:

Toggle quote (4 lines)
> I have my manifest.scm here:
> https://github.com/dongcarl/bitcoin/blob/2019-06-guix-channels-and-inferiors/contrib/guix/manifest.scm,
> and it references the aforementioned Guix channel.

This one appears to work for me (I interrupted it before it was done
compiling all the toolchains, though.)

Toggle quote (8 lines)
> What I expect to happen is that now when I change my default profile's Guix
> version by 'guix pull'ing, it won't affect the environment that's generated by
> the manifest.scm. I've tested this with differing versions of Guix as my default
> profile, and this seems to work with 7304d5623ab5cc35289cb1535cbd0d8a37691fac
> and 7f1c69f5d32bee6b8b6b902a9ce445e04aa9d07d being my default profile. However,
> I tried an older version b6dc08393e6a8313b88ce422fc3c1e4e9c0efc6f, and got the
> following error:

When I put b6dc08393e6a8313b88ce422fc3c1e4e9c0efc6f in .guix-channel and
use that as my channel, it also works fine (well, I commented out the
toolchains as well.)

Toggle quote (4 lines)
> ```
> guix environment: error: derivation `/gnu/store/r641vpqc9rfjhljf7rzfzwmkmpa642ls-info-dir.drv' has incorrect output `/gnu/store/q9hkdidycz3wq28xxgjq47bzx5s39k52-info-dir', should be `/gnu/store/z5hh2nl0h58b9f6hdxfwm00gjyxfcc3n-info-dir'
> ```

That definitely looks like a bug.

Could you send the faulty info-dir.drv file?

Thanks,
Ludo’.
C
C
Carl Dong wrote on 26 Jul 2019 21:58
(name . Ludovic Courtès)(address . ludo@gnu.org)(name . 36777@debbugs.gnu.org)(address . 36777@debbugs.gnu.org)
1WSz93jYl9XxYywM_b-gQYV4SiIG9VyTXLYpj4bueFcBZByRMIdXQNnVY4YDnbH_hPe4y_ZDjl7lIgyvynH8J85NtO93namyeEVMU7Lj9WY=@carldong.me
Hi all,

I did some more digging, and have included a git-bisect log, the -info-dir.drv,
and -info-dir-builder here:


Please let me know if I can provide more information.

Cheers,
Carl Dong
contact@carldong.me
"I fight for the users"

??????? Original Message ???????
On Friday, July 26, 2019 7:50 AM, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (32 lines)
> Hello Carl,
>
> Carl Dong contact@carldong.me skribis:
>
> > I have my manifest.scm here:
> > https://github.com/dongcarl/bitcoin/blob/2019-06-guix-channels-and-inferiors/contrib/guix/manifest.scm,
> > and it references the aforementioned Guix channel.
>
> This one appears to work for me (I interrupted it before it was done
> compiling all the toolchains, though.)
>
> > What I expect to happen is that now when I change my default profile's Guix
> > version by 'guix pull'ing, it won't affect the environment that's generated by
> > the manifest.scm. I've tested this with differing versions of Guix as my default
> > profile, and this seems to work with 7304d5623ab5cc35289cb1535cbd0d8a37691fac
> > and 7f1c69f5d32bee6b8b6b902a9ce445e04aa9d07d being my default profile. However,
> > I tried an older version b6dc08393e6a8313b88ce422fc3c1e4e9c0efc6f, and got the
> > following error:
>
> When I put b6dc08393e6a8313b88ce422fc3c1e4e9c0efc6f in .guix-channel and
> use that as my channel, it also works fine (well, I commented out the
> toolchains as well.)
>
> > guix environment: error: derivation `/gnu/store/r641vpqc9rfjhljf7rzfzwmkmpa642ls-info-dir.drv' has incorrect output `/gnu/store/q9hkdidycz3wq28xxgjq47bzx5s39k52-info-dir', should be `/gnu/store/z5hh2nl0h58b9f6hdxfwm00gjyxfcc3n-info-dir'
> >
>
> That definitely looks like a bug.
>
> Could you send the faulty info-dir.drv file?
>
> Thanks,
> Ludo’.
L
L
Ludovic Courtès wrote on 27 Jul 2019 23:30
(name . Carl Dong)(address . contact@carldong.me)(name . 36777@debbugs.gnu.org)(address . 36777@debbugs.gnu.org)
87wog3i2sj.fsf@gnu.org
Hi Carl,

Carl Dong <contact@carldong.me> skribis:

Toggle quote (5 lines)
> I did some more digging, and have included a git-bisect log, the -info-dir.drv,
> and -info-dir-builder here:
>
> https://gist.github.com/dongcarl/0a305badf20c9b5cfae738147ca416af

For future reference, the bisect output is:

# first bad commit: [5cf4b26d52bcea382d98fb4becce89be9ee37b55] derivations: <derivation-input> now aggregates a <derivation>.

and ‘info-dir.drv’ is attached below.

I looked at the offending commit. I don’t doubt it has the potential to
introduce such a bug :-), but so far I haven’t seen anything fishy as I
inspected it with a fresh eye.

Could you come up with a reduced reproducer for this? Ideally a small
manifest, the commit of the Guix you’re using (not just that of the
inferior), maybe a manifest that doesn’t even use inferiors, and so on.

Alternately, since the problem is that the output path of the derivation
is incorrectly computed, you could add ‘pk’ calls to print the value of
‘drv-masked’ in (guix derivations) or similar. That’s not going to be
convenient, though. :-/

Ludo’.

PS: I’ll be away from keyboard so I won’t be able to look into it until
some time.
Attachment: t.drv
L
L
Ludovic Courtès wrote on 27 Jul 2019 23:48
(name . Carl Dong)(address . contact@carldong.me)(name . 36777@debbugs.gnu.org)(address . 36777@debbugs.gnu.org)
87r26bi1x2.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (4 lines)
> For future reference, the bisect output is:
>
> # first bad commit: [5cf4b26d52bcea382d98fb4becce89be9ee37b55] derivations: <derivation-input> now aggregates a <derivation>.

Does the patch below have any effect?

My understanding is that it shouldn’t have any effect (because inputs
have already been coalesced at that point), but that’s one place where
5cf4b26d52bcea382d98fb4becce89be9ee37b55 introduced a difference.

Thanks in advance,
Ludo’.
Toggle diff (33 lines)
diff --git a/guix/derivations.scm b/guix/derivations.scm
index 92d50503ce..eb94fea55e 100644
--- a/guix/derivations.scm
+++ b/guix/derivations.scm
@@ -239,12 +239,17 @@ the store."
"Return a list of inputs, such that when INPUTS contains the same DRV twice,
they are coalesced, with their sub-derivations merged. This is needed because
Nix itself keeps only one of them."
+ (define (derivation-file-name* obj)
+ (if (derivation? obj)
+ (derivation-file-name obj)
+ obj))
+
(fold (lambda (input result)
(match input
- (($ <derivation-input> (= derivation-file-name path) sub-drvs)
+ (($ <derivation-input> (= derivation-file-name* path) sub-drvs)
;; XXX: quadratic
(match (find (match-lambda
- (($ <derivation-input> (= derivation-file-name p)
+ (($ <derivation-input> (= derivation-file-name* p)
s)
(string=? p path)))
result)
@@ -685,7 +690,7 @@ name of each input with that input's hash."
(make-derivation-input hash sub-drvs))))
inputs)))
(make-derivation outputs
- (sort inputs
+ (sort (coalesce-duplicate-inputs inputs)
(lambda (drv1 drv2)
(string<? (derivation-input-derivation drv1)
(derivation-input-derivation drv2))))
L
L
Ludovic Courtès wrote on 27 Jul 2019 23:49
control message for bug #36777
(address . control@debbugs.gnu.org)
87pnlvi1wt.fsf@gnu.org
severity 36777 important
quit
C
C
Carl Dong wrote on 30 Jul 2019 20:36
Re: bug#36777: Guix Inferiors: Curious incorrect derivation output bug
(name . Ludovic Courtès)(address . ludo@gnu.org)(name . 36777\@debbugs.gnu.org)(address . 36777@debbugs.gnu.org)
rvQNShnNXKeE-TRrrP9SzUqXX4dJEB481poZeW9Rh_RZ7tgj7BRuBFnV049S2IfmM6v32jY1T8WM_zn5ZDX-2Kyfw16jm4wSaRshHnntHJQ=@carldong.me
All,

Yes! The patch actually fixed the problem when applied on top of 5cf4b26d52bcea382d98fb4becce89be9ee37b55! I was also able to come up with a minimal reproducing manifest:

```
(use-modules (guix inferior) (guix channels)
(srfi srfi-1)) ;for 'first'

(define channels
(list (channel
(name 'guix)
(commit
"6869b6635afd93872b8f0d9f2db0db4c0d765a86"))))

(define inferior
;; An inferior representing the above revision.
(inferior-for-channels channels))

(packages->manifest
(map (lambda (x)
(first (lookup-inferior-packages inferior x)))
'("gzip")))
```

It seems that the `gzip` package is what's causing this.

Not sure what the next steps are for this, but I'd very much like to understand where this went wrong. Perhaps we could write tests for this so it doesn't happen in the future for releases.

Cheers,
Carl Dong
contact@carldong.me
"I fight for the users"
L
L
Ludovic Courtès wrote on 16 Aug 2019 23:03
(name . Carl Dong)(address . contact@carldong.me)(name . 36777@debbugs.gnu.org)(address . 36777-done@debbugs.gnu.org)
87h86gg6w4.fsf@gnu.org
Hi Carl,

Carl Dong <contact@carldong.me> skribis:

Toggle quote (2 lines)
> Yes! The patch actually fixed the problem when applied on top of 5cf4b26d52bcea382d98fb4becce89be9ee37b55!

[...]

Toggle quote (2 lines)
> Not sure what the next steps are for this, but I'd very much like to understand where this went wrong. Perhaps we could write tests for this so it doesn't happen in the future for releases.

Yup, I understood when this could happen (if multiple inputs of a
derivation are “fixed-output” derivations leading to the same output),
wrote a test for that, and came up with a simpler fix in commit
268896444bed7b958add74b2e1e86ff802c5f5cb.

Let me know if anything is amiss!

Thanks for testing the patch,
Ludo’.
Closed
?
Your comment

This issue is archived.

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

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