Snippets (even empty ones) of tar sources reset the timestamps of all files

  • Open
  • quality assurance status badge
Details
3 participants
  • Clément Lassieur
  • Ludovic Courtès
  • Mark H Weaver
Owner
unassigned
Submitted by
Clément Lassieur
Severity
normal

Debbugs page

C
C
Clément Lassieur wrote on 1 May 2017 06:57
(address . bug-guix@gnu.org)
874lx4d6j7.fsf@lassieur.org
Please consider the following package:

Toggle snippet (23 lines)
(define-public hello
(package
(name "hello")
(version "2.10")
(source (origin
(method url-fetch)
(uri (string-append "mirror://gnu/hello/hello-" version
".tar.gz"))
(sha256
(base32
"0ssi1wpaf7plaswqqjwigppsg5fyh99vdlb9kzl7c9lng89ndq1i"))
(modules '((guix build utils)))
(snippet '(begin #t))))
(build-system gnu-build-system)
(synopsis "Hello, GNU world: An example GNU package")
(description
"GNU Hello prints the message \"Hello, world!\" and then exits. It
serves as an example of standard GNU coding practices. As such, it supports
command-line arguments, multiple languages, and so on.")
(home-page "https://www.gnu.org/software/hello/")
(license gpl3+)))

It is the 'hello' package with two extra lines that add an empty
snippet.

Toggle snippet (4 lines)
(modules '((guix build utils)))
(snippet '(begin #t))

Now here is the output of 'ls -l' within the original (extracted)
'hello':

Toggle snippet (32 lines)
-rw-r--r-- 1 clement users 93787 Nov 16 2014 ABOUT-NLS
-rw-r--r-- 1 clement users 43830 Nov 16 2014 aclocal.m4
-rw-r--r-- 1 clement users 593 Jul 19 2014 AUTHORS
drwxr-xr-x 3 clement users 4096 Nov 16 2014 build-aux/
-rw-r--r-- 1 clement users 12988 Nov 16 2014 ChangeLog
-rw-r--r-- 1 clement users 30632 Jul 19 2014 ChangeLog.O
-rw-r--r-- 1 clement users 32871 Nov 16 2014 config.in
-rwxr-xr-x 1 clement users 449922 Nov 16 2014 configure
-rw-r--r-- 1 clement users 2408 Oct 5 2014 configure.ac
drwxr-xr-x 2 clement users 4096 Nov 16 2014 contrib/
-rw-r--r-- 1 clement users 35147 Dec 12 2013 COPYING
drwxr-xr-x 2 clement users 4096 Nov 16 2014 doc/
-rw-r--r-- 1 clement users 4573 Nov 16 2014 GNUmakefile
-r--r--r-- 1 clement users 1400 Nov 16 2014 hello.1
-rw-r--r-- 1 clement users 15752 Dec 29 2013 INSTALL
drwxr-xr-x 2 clement users 4096 Nov 16 2014 lib/
drwxr-xr-x 2 clement users 4096 Nov 16 2014 m4/
-rw-r--r-- 1 clement users 63223 Nov 16 2014 maint.mk
-rw-r--r-- 1 clement users 4367 Aug 5 2014 Makefile.am
-rw-r--r-- 1 clement users 143282 Nov 16 2014 Makefile.in
drwxr-xr-x 2 clement users 4096 Nov 16 2014 man/
-rw-r--r-- 1 clement users 4023 Nov 16 2014 NEWS
drwxr-xr-x 2 clement users 4096 Nov 16 2014 po/
-rw-r--r-- 1 clement users 3582 Jul 19 2014 README
-rw-r--r-- 1 clement users 1571 Jul 19 2014 README-dev
-rw-r--r-- 1 clement users 3014 Nov 16 2014 README-release
drwxr-xr-x 2 clement users 4096 Nov 16 2014 src/
drwxr-xr-x 2 clement users 4096 Nov 16 2014 tests/
-rw-r--r-- 1 clement users 898 Jul 19 2014 THANKS
-rw-r--r-- 1 clement users 75 Jul 19 2014 TODO

And here is the same output within the modified 'hello' (with the empty
snippet):

Toggle snippet (32 lines)
-rw-r--r-- 1 clement users 93787 Jan 1 1970 ABOUT-NLS
-rw-r--r-- 1 clement users 43830 Jan 1 1970 aclocal.m4
-rw-r--r-- 1 clement users 593 Jan 1 1970 AUTHORS
drwxr-xr-x 3 clement users 4096 Jan 1 1970 build-aux/
-rw-r--r-- 1 clement users 12988 Jan 1 1970 ChangeLog
-rw-r--r-- 1 clement users 30632 Jan 1 1970 ChangeLog.O
-rw-r--r-- 1 clement users 32871 Jan 1 1970 config.in
-rwxr-xr-x 1 clement users 449922 Jan 1 1970 configure
-rw-r--r-- 1 clement users 2408 Jan 1 1970 configure.ac
drwxr-xr-x 2 clement users 4096 Jan 1 1970 contrib/
-rw-r--r-- 1 clement users 35147 Jan 1 1970 COPYING
drwxr-xr-x 2 clement users 4096 Jan 1 1970 doc/
-rw-r--r-- 1 clement users 4573 Jan 1 1970 GNUmakefile
-r--r--r-- 1 clement users 1400 Jan 1 1970 hello.1
-rw-r--r-- 1 clement users 15752 Jan 1 1970 INSTALL
drwxr-xr-x 2 clement users 4096 Jan 1 1970 lib/
drwxr-xr-x 2 clement users 4096 Jan 1 1970 m4/
-rw-r--r-- 1 clement users 63223 Jan 1 1970 maint.mk
-rw-r--r-- 1 clement users 4367 Jan 1 1970 Makefile.am
-rw-r--r-- 1 clement users 143282 Jan 1 1970 Makefile.in
drwxr-xr-x 2 clement users 4096 Jan 1 1970 man/
-rw-r--r-- 1 clement users 4023 Jan 1 1970 NEWS
drwxr-xr-x 2 clement users 4096 Jan 1 1970 po/
-rw-r--r-- 1 clement users 3582 Jan 1 1970 README
-rw-r--r-- 1 clement users 1571 Jan 1 1970 README-dev
-rw-r--r-- 1 clement users 3014 Jan 1 1970 README-release
drwxr-xr-x 2 clement users 4096 Jan 1 1970 src/
drwxr-xr-x 2 clement users 4096 Jan 1 1970 tests/
-rw-r--r-- 1 clement users 898 Jan 1 1970 THANKS
-rw-r--r-- 1 clement users 75 Jan 1 1970 TODO

As you can see, the timestamps are not the same, although there is
nothing in the snippet.

I think the code is in guix/packages.scm (patch-and-repack). Here is my
understanding of what happens: the presence of the snippet leads to 3
things:

1. extraction of the archive,
2. modification of some files (because of patches or snippet),
3. compression of the archive.

Step 3 sets all timestamps to zero so to avoid non-determinism in the
archive. We obviously don't want archives to depend on the time of
their creation. But I believe we should only reset the timestamps of
the files we modified in step 2. Unmodified files should stay
unmodified.

What's the point? Well, while working on the 0ad package, I realized
that building with 32 cores would always fail with a snippet (even if it
is empty) and always succeed without a snippet. It took me a few hours
to understand this, and I put a comment in the 0ad package. Maybe it is
a 0ad bug (some tricky race condition that depends of file timestamps),
but I believe it is also a Guix bug: an empty snippet should not change
in any way the binary output of a package.

I tried to patch 'patch-and-repack', but it triggers a full
rebuild... WDYT?
L
L
Ludovic Courtès wrote on 2 May 2017 06:00
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 26734@debbugs.gnu.org)
87h913h0rj.fsf@gnu.org
Hi!

Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (14 lines)
> I think the code is in guix/packages.scm (patch-and-repack). Here is my
> understanding of what happens: the presence of the snippet leads to 3
> things:
>
> 1. extraction of the archive,
> 2. modification of some files (because of patches or snippet),
> 3. compression of the archive.
>
> Step 3 sets all timestamps to zero so to avoid non-determinism in the
> archive. We obviously don't want archives to depend on the time of
> their creation. But I believe we should only reset the timestamps of
> the files we modified in step 2. Unmodified files should stay
> unmodified.

I agree.

Toggle quote (8 lines)
> What's the point? Well, while working on the 0ad package, I realized
> that building with 32 cores would always fail with a snippet (even if it
> is empty) and always succeed without a snippet. It took me a few hours
> to understand this, and I put a comment in the 0ad package. Maybe it is
> a 0ad bug (some tricky race condition that depends of file timestamps),
> but I believe it is also a Guix bug: an empty snippet should not change
> in any way the binary output of a package.

Yeah I was bitten by the same problem recently.

Toggle quote (3 lines)
> I tried to patch 'patch-and-repack', but it triggers a full
> rebuild... WDYT?

Right, it’s expected to trigger a full rebuild, so this should be fixed
in ‘core-updates’.

I guess we’ll have to collect the timestamps of all non-symlink files¹
in step #1 and to reapply them with ‘set-file-time’ from (guix build
utils) after step #2.

Thoughts? Would you like to do that?

Thanks,
Ludo’.

¹ Because Guile provides bindings for ‘utime’, which does not support
setting timestamps on symlinks.
C
C
Clément Lassieur wrote on 2 May 2017 06:17
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 26734@debbugs.gnu.org)
87d1brmm8m.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (8 lines)
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> I tried to patch 'patch-and-repack', but it triggers a full
>> rebuild... WDYT?
>
> Right, it’s expected to trigger a full rebuild, so this should be fixed
> in ‘core-updates’.

Yes, but is there a way to test the patch on one package without having
first to rebuild everything?

Toggle quote (4 lines)
> I guess we’ll have to collect the timestamps of all non-symlink files¹
> in step #1 and to reapply them with ‘set-file-time’ from (guix build
> utils) after step #2.

Does that mean that symlinks will still have their timestamps changed?
To me that is a half-solution... Wouldn't it be easier to collect all
recently modified files (those modified by snippet and patches), and set
their timestamp to "1 January 1970", without changing the other files?
That means removing the --mtime option from tar at step 3.

Toggle quote (2 lines)
> Thoughts? Would you like to do that?

Sure :-)

Toggle quote (3 lines)
> ¹ Because Guile provides bindings for ‘utime’, which does not support
> setting timestamps on symlinks.

If the guile binding doesn't support setting timestamps on symlinks, I
guess we can still use another way, like a system touch.
L
L
Ludovic Courtès wrote on 3 May 2017 01:58
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 26734@debbugs.gnu.org)
87fugmxqng.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (13 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Clément Lassieur <clement@lassieur.org> skribis:
>>
>>> I tried to patch 'patch-and-repack', but it triggers a full
>>> rebuild... WDYT?
>>
>> Right, it’s expected to trigger a full rebuild, so this should be fixed
>> in ‘core-updates’.
>
> Yes, but is there a way to test the patch on one package without having
> first to rebuild everything?

I would add a snippet in ‘gnu-make-boot0’, which is the first package
being built, and then run:

./pre-inst-env guix build -S --rounds=2 \
-e '(@@ (gnu packages commencement) gnu-make-boot0)'

Toggle quote (6 lines)
>> I guess we’ll have to collect the timestamps of all non-symlink files¹
>> in step #1 and to reapply them with ‘set-file-time’ from (guix build
>> utils) after step #2.
>
> Does that mean that symlinks will still have their timestamps changed?

No, that means symlinks will still have their timestamps unchanged. :-)

BTW, what timestamps to we put on the modified files? We want that to
be deterministic so we cannot use the build time. We cannot use a date
in the future, either. We cannot use Jan. 1 1970 either because that
means that modified files may now be older than the unmodified files,
which may break build systems; for the same reason, we cannot leave the
mtime of modified files unchanged.

Now that I think about it, it’s not clear to me what can be done without
breaking something.

Thoughts?

Toggle quote (6 lines)
>> ¹ Because Guile provides bindings for ‘utime’, which does not support
>> setting timestamps on symlinks.
>
> If the guile binding doesn't support setting timestamps on symlinks, I
> guess we can still use another way, like a system touch.

Or we could add bindings for ‘futimes’.

Thanks,
Ludo’.
M
M
Mark H Weaver wrote on 3 May 2017 14:45
(name . Ludovic Courtès)(address . ludo@gnu.org)
87r305iphr.fsf@netris.org
ludo@gnu.org (Ludovic Courtès) writes:

Toggle quote (12 lines)
> BTW, what timestamps to we put on the modified files? We want that to
> be deterministic so we cannot use the build time. We cannot use a date
> in the future, either. We cannot use Jan. 1 1970 either because that
> means that modified files may now be older than the unmodified files,
> which may break build systems; for the same reason, we cannot leave the
> mtime of modified files unchanged.
>
> Now that I think about it, it’s not clear to me what can be done without
> breaking something.
>
> Thoughts?

We could set the timestamp of modified files to be 1 second newer than
the newest file in the original source archive.

Mark
L
L
Ludovic Courtès wrote on 3 May 2017 15:01
(name . Mark H Weaver)(address . mhw@netris.org)
87r305ppla.fsf@gnu.org
Mark H Weaver <mhw@netris.org> skribis:

Toggle quote (17 lines)
> ludo@gnu.org (Ludovic Courtès) writes:
>
>> BTW, what timestamps to we put on the modified files? We want that to
>> be deterministic so we cannot use the build time. We cannot use a date
>> in the future, either. We cannot use Jan. 1 1970 either because that
>> means that modified files may now be older than the unmodified files,
>> which may break build systems; for the same reason, we cannot leave the
>> mtime of modified files unchanged.
>>
>> Now that I think about it, it’s not clear to me what can be done without
>> breaking something.
>>
>> Thoughts?
>
> We could set the timestamp of modified files to be 1 second newer than
> the newest file in the original source archive.

Sounds like a good idea.

Thanks,
Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 26734@patchwise.org

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