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. Itserves as an example of standard GNU coding practices. As such, it supportscommand-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 emptysnippet.
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 AUTHORSdrwxr-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.acdrwxr-xr-x 2 clement users 4096 Nov 16 2014 contrib/-rw-r--r-- 1 clement users 35147 Dec 12 2013 COPYINGdrwxr-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 INSTALLdrwxr-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.indrwxr-xr-x 2 clement users 4096 Nov 16 2014 man/-rw-r--r-- 1 clement users 4023 Nov 16 2014 NEWSdrwxr-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-releasedrwxr-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 emptysnippet):
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 AUTHORSdrwxr-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.acdrwxr-xr-x 2 clement users 4096 Jan 1 1970 contrib/-rw-r--r-- 1 clement users 35147 Jan 1 1970 COPYINGdrwxr-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 INSTALLdrwxr-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.indrwxr-xr-x 2 clement users 4096 Jan 1 1970 man/-rw-r--r-- 1 clement users 4023 Jan 1 1970 NEWSdrwxr-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-releasedrwxr-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 isnothing in the snippet.
I think the code is in guix/packages.scm (patch-and-repack). Here is myunderstanding of what happens: the presence of the snippet leads to 3things:
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 thearchive. We obviously don't want archives to depend on the time oftheir creation. But I believe we should only reset the timestamps ofthe files we modified in step 2. Unmodified files should stayunmodified.
What's the point? Well, while working on the 0ad package, I realizedthat building with 32 cores would always fail with a snippet (even if itis empty) and always succeed without a snippet. It took me a few hoursto understand this, and I put a comment in the 0ad package. Maybe it isa 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 changein any way the binary output of a package.
I tried to patch 'patch-and-repack', but it triggers a fullrebuild... 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 fixedin ‘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 buildutils) 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 havingfirst 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 allrecently modified files (those modified by snippet and patches), and settheir 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, Iguess 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 packagebeing 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 tobe deterministic so we cannot use the build time. We cannot use a datein the future, either. We cannot use Jan. 1 1970 either because thatmeans that modified files may now be older than the unmodified files,which may break build systems; for the same reason, we cannot leave themtime of modified files unchanged.
Now that I think about it, it’s not clear to me what can be done withoutbreaking 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 thanthe 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’.
?