add configureReproducible( Date lastModifiedDate ) API #121
Conversation
|
I will have a look at this tonight. |
| protected Date convertSourceDateEpochToDate( int sourceDateEpoch ) | ||
| { | ||
| // timestamp of zip entries at zip storage level ignores timezone: see https://github.com/Stuk/jszip/issues/369 | ||
| Calendar cal = Calendar.getInstance(); |
There was a problem hiding this comment.
This one is to trick the default timezone version, right?
There was a problem hiding this comment.
"trick the default timezone": I'm not ok with this way of saying things, but it looks it's your way to say what I wrote in comments
There was a problem hiding this comment.
But that's what you do. ZipEntry.setTime() does use the default timezone. You recalculate the date to have it look like UTC for the ZipOutputStream. Therefore your comment is not correct. The ZIP defition has no timezone notion, but many implementations use the default timezone. The provided link describes how to defeat the system.
There was a problem hiding this comment.
I still don't see what to do with your comment
There was a problem hiding this comment.
Let's put the sourceEpochDate aside. All you do is to modify the timestamp to look like UTC in the default timezone, don't you? It has nothing to do with the sourceEpochDate. You fix a shortcoming of the ZIP spec. Simply convert to UTC. That is what you need to document.
src/main/java/org/codehaus/plexus/archiver/AbstractArchiver.java
Outdated
Show resolved
Hide resolved
hboutemy
force-pushed
the
reproducible
branch
from
c8c8f01
to
456e4f6
Compare
There was a problem hiding this comment.
A few glitches, but overall a good thing. One though, is missing. The TZ alignment cannot be done w/o using the configure method because the former is protected. A user can actually make the archive reproducible w/o using the configure method. He ought to normalize the timestamp himself in this case.
src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipArchiver.java
Outdated
Show resolved
Hide resolved
| } | ||
| } ); | ||
|
|
||
| // 3. ignore file/directory mode from filesystem, since they may vary based on local user umask |
There was a problem hiding this comment.
This can be quite inconvenient if the archive contains executable scripts (not uncommon for Java applications). I think that at least it should be explicitly stated in the configureReproducible java docs - "reproducible entries Unix mode" is not specific enough. Also I was thinking if it would make sense to keep the executable flag. Having a umask with the executable flag set to true on Unix system is quite uncommon.
There was a problem hiding this comment.
yes, executable bit is the most wanted one.
Not sure this bit is really managed when running on Windows.
I don't know what precisely to do
I'll merge the current state as it is a good initial step: please help by providing a PR to improve
hboutemy
changed the title
hboutemy
force-pushed
the
reproducible
branch
from
b1201ec
to
ab1f0a9
Compare
src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipArchiver.java
Outdated
Show resolved
Hide resolved
code extracted from POC done in Maven Source Plugin MSOURCES-120
now need to test it from Maven Assembly Plugin to see how the proposed API matches more advanced use cases (other formats than jar, execution bit in Unix file mode, perhaps other aspects)