GNU bug report logs - #31560
Commit 47a60325c broke tests/pack.scm

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31560 in the body.
You can then email your comments to 31560 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Chris Marusich <cmmarusich <at> gmail.com>
To: bug-guix <at> gnu.org
Subject: Commit 47a60325c broke tests/pack.scm
Date: Tue, 22 May 2018 23:09:58 -0700

[Message part 1 (text/plain, inline)]

Hi,

It seems that tests/pack.scm began to fail at commit
47a60325ca650e8fc1a291c8655b4297f4de8deb.  This commit made the
following change in guix/scripts/pack.scm:

--8<---------------cut here---------------start------------->8---
           (define symlink->directives
             ;; Return "populate directives" to make the given symlink and its
             ;; parent directories.
             (match-lambda
               ((source '-> target)
                (let ((target (string-append #$profile "/" target)))
                  `((directory ,(dirname source))
-                   (,source -> ,target))))))
+                   (,source
+                    -> ,(relative-file-name (dirname source) target)))))))
--8<---------------cut here---------------end--------------->8---

It seems this causes the test to fail because it calls
relative-file-name like this in the test:

(relative-file-name "/bin"
"/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile")

which evaluates to

"../home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile"

which is not equal to

"/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile"

so the test fails.

What is the purpose of calling relative-file-name here?

-- 
Chris

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 31560 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Chris Marusich <cmmarusich <at> gmail.com>
Cc: 31560 <at> debbugs.gnu.org
Subject: Re: bug#31560: Commit 47a60325c broke tests/pack.scm
Date: Wed, 23 May 2018 09:46:03 +0200

Hi Chris,

Chris Marusich <cmmarusich <at> gmail.com> skribis:

> It seems that tests/pack.scm began to fail at commit
> 47a60325ca650e8fc1a291c8655b4297f4de8deb.  This commit made the
> following change in guix/scripts/pack.scm:
>
>            (define symlink->directives
>              ;; Return "populate directives" to make the given symlink and its
>              ;; parent directories.
>              (match-lambda
>                ((source '-> target)
>                 (let ((target (string-append #$profile "/" target)))
>                   `((directory ,(dirname source))
> -                   (,source -> ,target))))))
> +                   (,source
> +                    -> ,(relative-file-name (dirname source) target)))))))
>
> It seems this causes the test to fail because it calls
> relative-file-name like this in the test:
>
> (relative-file-name "/bin"
> "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile")
>
> which evaluates to
>
> "../home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile"
>
> which is not equal to
>
> "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile"
>
> so the test fails.

Oooh, sorry about that.

However what happens here is more something like:

  (relative-file-name
    "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin"
    "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile")

No?  Hmm not sure…

I suppose the test should ensure we get an appropriate relative symlink.

> What is the purpose of calling relative-file-name here?

The goal is to create only relative symlinks so that the tarball is
relocatable:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31360

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix pack bash-static -S /bin/B=bin/bash
/gnu/store/7dks90572s5i8nnziippsskm7f4iyqhg-tarball-pack.tar.gz
$ tar tvf /gnu/store/7dks90572s5i8nnziippsskm7f4iyqhg-tarball-pack.tar.gz |grep bin/B
lrwxrwxrwx root/root         0 1970-01-01 01:00 ./bin/B -> ../gnu/store/xqgyj976y375wv8gaqh5mz0ysbfdk7f6-profile/bin/bash
hrwxrwxrwx root/root         0 1970-01-01 01:00 ./bin/B link to ./bin/B
--8<---------------cut here---------------end--------------->8---

(The “link to” line really shouldn’t be here but let’s ignore it.)

HTH!

Ludo’.

Message #11 received at 31560 <at> debbugs.gnu.org (full text, mbox):

From: Chris Marusich <cmmarusich <at> gmail.com>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 31560 <at> debbugs.gnu.org
Subject: Re: bug#31560: Commit 47a60325c broke tests/pack.scm
Date: Fri, 01 Jun 2018 00:45:34 -0700

[Message part 1 (text/plain, inline)]

Hi Ludo,

I believe the attached patches fix the issue when applied to the master
branch.  The failing test now passes (along with all the other "make
check" tests).  What do you think?  If you approve, then I'll commit it
to the master branch.

ludo <at> gnu.org (Ludovic Courtès) writes:

> Chris Marusich <cmmarusich <at> gmail.com> skribis:
>
>> It seems that tests/pack.scm began to fail at commit
>> 47a60325ca650e8fc1a291c8655b4297f4de8deb.  This commit made the
>> following change in guix/scripts/pack.scm:
>>
>>            (define symlink->directives
>>              ;; Return "populate directives" to make the given symlink and its
>>              ;; parent directories.
>>              (match-lambda
>>                ((source '-> target)
>>                 (let ((target (string-append #$profile "/" target)))
>>                   `((directory ,(dirname source))
>> -                   (,source -> ,target))))))
>> +                   (,source
>> +                    -> ,(relative-file-name (dirname source) target)))))))
>>
>> It seems this causes the test to fail because it calls
>> relative-file-name like this in the test:
>>
>> (relative-file-name "/bin"
>> "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile")
>>
>> ...
>
> Oooh, sorry about that.

No worries!  Hopefully the fix is as simple as I think it is.

> However what happens here is more something like:
>
>   (relative-file-name
>     "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin"
>     "/home/marusich/guix-upgrade-gnucash/test-tmp/store/3j3mrl1sf3bcx4fzlz655mzsp4bir54j-profile/bin/guile")
>
> No?  Hmm not sure…

On my system, when I ran the test (tests/pack.scm) just now, the
variables in symlink->directives had the following values (I used pk to
view their values - if you know of a better way, I'd love to know!):

  source = "/bin/Guile"
  target = "/home/marusich/guix/test-tmp/store/p8y0nby1mybh5h75hi4hd1x0x36insnd-profile/bin/guile"
  parent = "/bin"

I believe the issue here was that we were not distinguishing between the
"source parent" and the "target parent" directories.  When calling
relative-file-name, I think we need to pass in the "target parent",
which I've done in the attached patches.

> I suppose the test should ensure we get an appropriate relative
>symlink.

Well, the test did catch the problem, so I think the test is OK as
currently written.

>> What is the purpose of calling relative-file-name here?
>
> The goal is to create only relative symlinks so that the tarball is
> relocatable:
>
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31360
>
> $ ./pre-inst-env guix pack bash-static -S /bin/B=bin/bash
> /gnu/store/7dks90572s5i8nnziippsskm7f4iyqhg-tarball-pack.tar.gz
> $ tar tvf /gnu/store/7dks90572s5i8nnziippsskm7f4iyqhg-tarball-pack.tar.gz |grep bin/B
> lrwxrwxrwx root/root         0 1970-01-01 01:00 ./bin/B -> ../gnu/store/xqgyj976y375wv8gaqh5mz0ysbfdk7f6-profile/bin/bash
> hrwxrwxrwx root/root         0 1970-01-01 01:00 ./bin/B link to ./bin/B
>
> (The “link to” line really shouldn’t be here but let’s ignore it.)

Understood!  Thank you for clarifying the intent.  I figured it was
something like that.  I just wanted to be certain.

-- 
Chris

[0001-tests-Call-self-contained-tarball-correctly.patch (text/x-patch, attachment)]

[0002-pack-Create-the-populate-directives-correctly.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #16 received at 31560-done <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Chris Marusich <cmmarusich <at> gmail.com>
Cc: 31560-done <at> debbugs.gnu.org
Subject: Re: bug#31560: Commit 47a60325c broke tests/pack.scm
Date: Fri, 01 Jun 2018 15:41:46 +0200

Hello,

Chris Marusich <cmmarusich <at> gmail.com> skribis:

> I believe the attached patches fix the issue when applied to the master
> branch.  The failing test now passes (along with all the other "make
> check" tests).  What do you think?  If you approve, then I'll commit it
> to the master branch.

I looked some more and the patch would create broken links.  I concluded
that the issue was simply that the test needed to be adjusted to expect
a relative symlink instead of an absolute symlink.

I first cherry-picked the fix that Ricardo made in ‘core-updates’ (same
as the one you posted) as commit
44057a461b1fa8102938c4e0f54d7cbc9dd09b03.  Then I adjusted the test in
commit ccc951cab3172adfdaf6fd2dfa8f8cdb98358a69.

Let me know if you think something is amiss!

Thank you,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.