tytso / e2fsprogs

Ext2/3/4 file system utilities
http://ext4.wiki.kernel.org
373 stars 219 forks source link

`E2FSPROGS_FAKE_TIME` not respected when setting time to 0 #184

Closed katexochen closed 4 months ago

katexochen commented 5 months ago

Setting E2FSPROGS_FAKE_TIME=0 will lead to the current time being used.

See for example the current line (total 3 findings of similar statements):

https://github.com/tytso/e2fsprogs/blob/260dfea450e387cbd2c8de79a7c2eeacc26f74e9/lib/ext2fs/inode.c#L1045

The check will handle E2FSPROGS_FAKE_TIME=0 as if E2FSPROGS_FAKE_TIME is unset.

It is common to set times to unix zero for reproducible builds.

Related to #131 and #164.

tytso commented 5 months ago

It is common to set times to unix zero for reproducible builds.

Citation needed.

In any case, there is no clean way of doing what you want, so if you want to do "reproducible builds", as the old joke goes, "Doctor, doctor, it hurts when I do this! // Then don't do that!". I will note that in the SOURCE_DATE_EPOCH specification, the recommendation is not to use zero, but...

Distributions could set SOURCE_DATE_EPOCH by using the value of a changelog file. For instance, Debian packages can set it to the value of the latest entry of debian/changelog.

Developers could set SOURCE_DATE_EPOCH to the date of the latest commit in their version control system. In this case, it is recommended to also update all source file timestamps, which git does not do by itself, otherwise old timestamps specific to the developer's working tree may be embedded into the output.

katexochen commented 5 months ago

I said it is common, not that it is recommended. ;-) It is just something you run into easily and it's hard to figure out what's going on. And while there is nothing forcing us to implement this, it is a nice move not letting fellow devs run into the open knife.

In any case, there is no clean way of doing what you want

Not sure why this can't be done in a clean way. The C code snippet suggested by Reproducible Builds handles that case as far as I can tell. Just write that into a function, make it also handle E2FSPROGS_FAKE_TIME env var and call it in the places where we currently do t = fs->now ? fs->now : time(NULL);.

If you like, I can open a PR.

tytso commented 5 months ago

The library interface for libext2fs is fs->now. What you are proposing would require breaking that interface and hacking the environment variable check into libext2fs. Hence, that is not clean; it would be a hack.

katexochen commented 5 months ago

I understand the interface needs to stay the same. Why is it hacky to add the check into libext2fs? We could check for SOURCE_DATE_EPOCH, and set it the way recommended by reproducible builds org. If it is unset, we can use the current fs->now ? fs->now : time(NULL). This should be backwards compatible, as it only changes the behavior in case SOURCE_DATE_EPOCH is set.

tytso commented 4 months ago

SOURCE_DATE_EPOCH has been implementing with timestamp clamping in v1.47.1-rc1+ . This should provide the functionality you want.