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

OpenSubmitted by Clément Lassieur.
Details
3 participants
  • Clément Lassieur
  • Ludovic Courtès
  • Mark H Weaver
Owner
unassigned
Severity
normal
C
C
Clément Lassieur wrote on 1 May 2017 15: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 15: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 15: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 10: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 23: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 4 May 2017 00: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’.
?