Bug 252308

Summary:	REGRESSION(256816@main): [CMake] Build not reproducible due to __TIMESTAMP__ macro in generated file
Product:	WebKit	Reporter:	Alberto Garcia <berto>
Component:	JavaScriptCore	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED DUPLICATE
Severity:	Normal	CC:	alex.kanavin, bugs-noreply, emw, mcatanzaro
Priority:	P2
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
See Also:	https://bugs.webkit.org/show_bug.cgi?id=247622

Description Alberto Garcia 2023-02-15 03:02:37 PST

The JSCBytecodeCacheVersion.cpp file contains the __TIMESTAMP__ macro
which is replaced by the timestamp of the source file. This macro was
already being used in 2.38.x and earlier versions, but at least
building always from the same source tarball would produce the same
timestamp so it would not affect the reproducibility of the build.

However this is now generated by Source/JavaScriptCore/CMakeLists.txt
so the timestamp varies every time WebKit is compiled from the same
sources, making the build non-reproducible.

Comment 1 Michael Catanzaro 2023-02-15 06:53:28 PST

Indeed, __TIMESTAMP__ is only deterministic when the source file is not generated, because it's based on last modification of the source file. The new comment in JSCBytecodeCacheVersion.h is also wrong except on Apple ports.

The solution here should be to do the same thing Apple does: cat JSCBuiltins.o with CachedTypes.o and then hash that. They also use build version, but I'm not sure why and that can probably be ignored. This would likely be a good newcomers bug as it can be done entirely from CMake with no need to touch the source code files.

Comment 2 Elliott Williams 2023-02-15 16:25:21 PST

(In reply to Michael Catanzaro from comment #1)
> Indeed, __TIMESTAMP__ is only deterministic when the source file is not
> generated, because it's based on last modification of the source file. The
> new comment in JSCBytecodeCacheVersion.h is also wrong except on Apple ports.
> 
> The solution here should be to do the same thing Apple does: cat
> JSCBuiltins.o with CachedTypes.o and then hash that.

> They also use build version, but I'm not sure why and that can probably be ignored. 

We did this for extra safety, in case our production build environment was using a cached JSCBuiltins.o and CachedTypes.o *and* there was some binary incompatibility elsewhere in JSC.

> This would
> likely be a good newcomers bug as it can be done entirely from CMake with no
> need to touch the source code files.

FWIW, I tried to do the same change in CMake, and got stuck because the generated ninja build doesn't support inter-dependencies between object files in the same target. But I think we could make CMake hash the modification time relatively easy and restore the old semantics.

Comment 3 Michael Catanzaro 2023-06-26 13:56:46 PDT

*** Bug 258517 has been marked as a duplicate of this bug. ***

Comment 4 Alexander Kanavin 2023-06-26 14:09:34 PDT

To at least restore the binary reproducibility I made a PR where mtime of the generated file is set to the one it is generated from using 'touch -r':

https://github.com/WebKit/WebKit/pull/15293

Comment 5 Michael Catanzaro 2023-08-10 14:22:42 PDT


*** This bug has been marked as a duplicate of bug 258517 ***