| Summary: | Make build of ghostscript deterministic | ||
|---|---|---|---|
| Product: | Ghostscript | Reporter: | Stefan Brüns <stefan.bruens> |
| Component: | Build Process | Assignee: | Chris Liddell (chrisl) <chris.liddell> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | jsmeix, stefan.bruens |
| Priority: | P4 | ||
| Version: | unspecified | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Customer: | Word Size: | --- | |
| Attachments: |
Use fixed time in timestamp
Sort resources by name Sort resources by name Patch v2 for real |
||
|
Description
Stefan Brüns
2017-01-16 14:54:32 UTC
Created attachment 13300 [details]
Sort resources by name
The sorting resources patch is fine by me, so I'll put that in soon. I know it's considered "standard" now, but I have to say that I am *fundamentally* opposed to the SOURCE_DATE_EPOCH thing (not just for us, but in general). If a build process is specifically designed to include the current date, it should include the current date. A "solution" that allows the real time of the build to replaced with any random date is potentially just misleading. FWIW, I'm not at all sure *why* we embed the date in the romfs. My first step is going to be to establish why. If there's no good reason, then we'll just remove the date stamp. If there *is* a good reason then, I'd be tempted to check for the existence of SOURCE_DATE_EPOCH and if it is set, then set the date to all zeros - that way, there can be *no* confusion that the date is "real". (In reply to Chris Liddell (chrisl) from comment #2) > The sorting resources patch is fine by me, so I'll put that in soon. Just a small Heads Up, the patch omits freeing the string array. For mkromfs this is likely not a big issue, but Valgrind or some static code checkers may be unhappy about that. > I know it's considered "standard" now, but I have to say that I am > *fundamentally* opposed to the SOURCE_DATE_EPOCH thing (not just for us, but > in general). If a build process is specifically designed to include the > current date, it should include the current date. A "solution" that allows > the real time of the build to replaced with any random date is potentially > just misleading. Unfortunately most build processes are not aware they are embedding a semi-random timestamp (depending on existence of some sort of real-time clock, speed of harddisk, ...). > FWIW, I'm not at all sure *why* we embed the date in the romfs. My first > step is going to be to establish why. If there's no good reason, then we'll > just remove the date stamp. If there *is* a good reason then, I'd be tempted > to check for the existence of SOURCE_DATE_EPOCH and if it is set, then set > the date to all zeros - that way, there can be *no* confusion that the date > is "real". *If* there currently is a reason for the timestamp beyond being pure informational, this dependency should be removed, as it is prone to break anyway, in misterious ways. Created attachment 13336 [details]
Sort resources by name
v2: free strduped string, file list
Regarding the timestamp in the romfs:
1. It is used in base/gsiorom.c to determine if the romfs is a dummy. This could be changed to "if (!gs_romfs[0]) { ...", as the first entry in the dummy rom is initialized to zero.
2. It is passed on in the st_mtime/st_ctime fields, and later used by zstatus in psi/zfile.c. zstatus itself does not interpret the fields, but puts the the values on the interpreter stack.
As the interpretation of the mtime/ctime is not really specified (according to PSRM), I would say filling in 0/0 would be ok.
OKay, sorry to mess you around on this. The more I've thought about it, the more I'm okay (not happy, exactly, but "okay") with using SOURCE_DATE_EPOCH in this instance. So, I am inclined to accept the patches as they stand. Patches applied in: http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=98696f718b http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=aa28186288 (In reply to Chris Liddell (chrisl) from comment #7) > Patches applied in: > > http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=98696f718b > http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=aa28186288 The second patch leaks memory, I had sent a second version of the patch which cleans up allocated memory. (In reply to Stefan Brüns from comment #8) > (In reply to Chris Liddell (chrisl) from comment #7) > > Patches applied in: > > > > http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=98696f718b > > http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=aa28186288 > > The second patch leaks memory, I had sent a second version of the patch > which cleans up allocated memory. Damn, I did use the second version of the "Sort resources by name" patch - looking at the two, the obsolete patch and the current one are identical. This should be sufficient: http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=5b196e5f Created attachment 13366 [details]
Patch v2 for real
Did upload the wrong file, this one should be correct
In the interest of clarity and maintainability, I prefer having it this way: https://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=c7d5567fbc |