Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SOURCE_DATE_EPOCH=0 not clamping file mtime #2679

Closed
champtar opened this issue Sep 27, 2023 · 5 comments · Fixed by #2756
Closed

SOURCE_DATE_EPOCH=0 not clamping file mtime #2679

champtar opened this issue Sep 27, 2023 · 5 comments · Fixed by #2756
Assignees
@dmnks
Labels
REGRESSION

Comments

@champtar
Copy link

Here a simple reproducer:

Name:    test
Version: 1
Release: 1
Summary: test SOURCE_DATE_EPOCH=0
License: GPLv2

%global source_date_epoch_from_changelog 0
%global clamp_mtime_to_source_date_epoch 1
%global use_source_date_epoch_as_buildtime 1

%description

%build
echo "this is a test" > 0.txt

%install
%{__install} -m 644 -D 0.txt %{buildroot}/0.txt

%files
/0.txt

$ SOURCE_DATE_EPOCH=0 rpmbuild -bb test.spec
$ rpm -q --dump ~/rpmbuild/RPMS/x86_64/test-1-1.x86_64.rpm
/0.txt 15 1695843332 91751cee0a1ab8414400238a761411daa29643ab4b8243e9a91649e25be53ada 0100644 root root 0 0 0 X
$ rpm -qi ~/rpmbuild/RPMS/x86_64/test-1-1.x86_64.rpm
Build Date  : Wed Dec 31 19:00:00 1969

It works fine with SOURCE_DATE_EPOCH=1

$ SOURCE_DATE_EPOCH=1 rpmbuild -bb test.spec
$ rpm -q --dump ~/rpmbuild/RPMS/x86_64/test-1-1.x86_64.rpm
/0.txt 15 1 91751cee0a1ab8414400238a761411daa29643ab4b8243e9a91649e25be53ada 0100644 root root 0 0 0 X

This is an issue on both Fedora 38 (rpm-4.18.1-3.fc38.x86_64) and Alma 9 (rpm-4.16.1.3-22.el9.x86_64)
It works fine on Alma 8 (rpm-4.14.3-26.el8.x86_64)

$ SOURCE_DATE_EPOCH=0 rpmbuild -bb test.spec
$ rpm -q --dump ~/rpmbuild/RPMS/x86_64/test-1-1.x86_64.rpm
/0.txt 15 0 91751cee0a1ab8414400238a761411daa29643ab4b8243e9a91649e25be53ada 0100644 root root 0 0 0 X
@champtar
Copy link
Author

champtar commented Sep 27, 2023 •
edited

Looking at the commits, it's likely caused by 11132fc, ping @pmatilai

@pmatilai
Copy link
Member

Well yes, that commit does turn value 0 into "disabled" as is common in rpm. As it says in the commit message.

If it's a SOURCE_DATE_EPOCH spec violation then please point out the relevant section, I guess we need to do something about it (it's not exactly hard to fix). Otherwise I have a hard time seeing ability to set time to that one second in the seventies as a particularly important real-world use-scenario.

@champtar
Copy link
Author

I build rpm to be used by rpm-ostree, rpm-ostree set all dates to 0 that's why I picked 0 in the past.

Using 1 or any other values should work for me, but I think SOURCE_DATE_EPOCH=0 is valid and it use to work, so if we don't fix it we should document it or warn about it.

@pmatilai
Copy link
Member

rpm-ostree set all dates to 0

What? 😳 Why would it do that? @cgwalters

But okay, it is actually used for whatever reasons I do not fathom, so I guess we should fix this.

@cgwalters
Copy link
Contributor

ostree always uses zero for mtime of files it writes because there are no timestamps in the file format at all. And in order to have sharing via hardlinks, there's then the question of what time to apply to that inode.

If there was a way in a Unix filesystem to have no timestamp at all, we'd definitely do that! In Rust terms, ideally stat() would return Option<time_t>.

Longer term though, we're moving towards https://github.com/containers/composefs which will give us in-memory/on-disk sharing even if we just let file timestamps float to "whatever", which will be helpful here. Also, it's likely in that world that we could consider switching to canonicalizing the on-disk (in EROFS) timestamp to e.g. something like the overall build's SOURCE_DATE_EPOCH, which could definitely be non-zero.

@dmnks dmnks self-assigned this Nov 2, 2023
dmnks added a commit to dmnks/rpm that referenced this issue Nov 8, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
74e2218 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
dmnks added a commit to dmnks/rpm that referenced this issue Nov 8, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
adaa655 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
dmnks added a commit to dmnks/rpm that referenced this issue Nov 8, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
dd866d1 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
@dmnks dmnks mentioned this issue Nov 8, 2023
Merged
pmatilai pushed a commit that referenced this issue Nov 9, 2023
@dmnks @pmatilai
Allow SOURCE_DATE_EPOCH=0 again
bb1eeb4 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: #2679
dmnks added a commit to dmnks/rpm that referenced this issue Nov 10, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
6e14d41 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
(backported from commit bb1eeb4)
dmnks added a commit to dmnks/rpm that referenced this issue Nov 13, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
c716f73 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
(backported from commit bb1eeb4)
dmnks added a commit that referenced this issue Nov 13, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
2c47442 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: #2679
(backported from commit bb1eeb4)
dmnks added a commit to dmnks/rpm that referenced this issue Nov 28, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
ee62c2e 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
(cherry picked from commit bb1eeb4)
dmnks added a commit to dmnks/rpm that referenced this issue Nov 28, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
1e4420d 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: rpm-software-management#2679
(cherry picked from commit bb1eeb4)
dmnks added a commit that referenced this issue Dec 12, 2023
@dmnks
Allow SOURCE_DATE_EPOCH=0 again
30f222c 
Commit 11132fc assumed that the value
of 0 is never used in practice and thus used it to indicate "disabled",
however that assumption has turned out to be wrong because ostree uses
precisely that value as mtime in inodes, which in turn breaks existing
workflows in this space (see the associated ticket).

Fix this by reverting the above commit (except leaving source_date_epoch
initialized to 0, to prevent GCC warnings as mentioned in that commit).

As to why not just initialize source_date_epoch to -1: time_t happens to
be a signed integer on most platforms but POSIX doesn't specify its
signed-ness.

Add some accompanying tests too.

Fixes: #2679
(cherry picked from commit bb1eeb4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmnks dmnks

Labels
REGRESSION
Projects
RPM
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow SOURCE_DATE_EPOCH=0 again dmnks/rpm
4 participants
@cgwalters @champtar @pmatilai @dmnks