gnodet-bot commented on code in PR #599:
URL: https://github.com/apache/maven-jar-plugin/pull/599#discussion_r4089163235
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -50,6 +50,20 @@
* @author Martin Desruisseaux
*/
public abstract class AbstractJarMojo implements
org.apache.maven.api.plugin.Mojo {
+ /**
+ * Minimum Unix time (seconds since epoch) accepted by the {@code jar}
tool.
+ * The {@code jar} tool enforces that all ZIP entry timestamps fall within
the range
+ * {@code 1980-01-01T00:00:02Z} to {@code 2099-12-31T23:59:59Z}.
+ * The lower bound is {@code T00:00:02Z} rather than {@code T00:00:00Z}
because
+ * {@code 1980-01-01T00:00:00Z} is a sentinel value ({@code
DOSTIME_BEFORE_1980}) in the
+ * JDK ZIP implementation that causes extra timezone metadata to be
written, breaking
+ * reproducibility (see JDK-8246129). The next instant after that sentinel
representable
+ * in MS-DOS time (which has 2-second granularity) is {@code T00:00:02Z}.
Review Comment:
⚠️ **Javadoc inaccuracy — the ‘2-second granularity’ explanation is wrong**
The text says: *“The next instant after that sentinel representable in
MS-DOS time (which has 2-second granularity) is `T00:00:02Z`.”*
This is incorrect. MS-DOS time encodes seconds as `seconds ÷ 2` in a 5-bit
field, so `00:00:00` (field value 0) **is** representable. The 2-second
granularity does not prevent `T00:00:00` — it only means `T00:00:01` cannot be
stored.
The real reason `T00:00:02Z` is the floor (per openjdk/jdk#6481) is that the
JDK explicitly chose it to avoid ambiguity with the `DOSTIME_BEFORE_1980`
sentinel: `T00:00:00Z` is the sentinel, `T00:00:01Z` is not representable in
DOS time, so `T00:00:02Z` is the lowest safe value.
Suggested replacement for lines 60–61:
```
* reproducibility (see JDK-8246129). The value {@code T00:00:01Z} is
not representable
* in MS-DOS time (2-second granularity), so {@code T00:00:02Z} is the
lowest safe value.
```
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -270,13 +292,31 @@ protected String getOutputTimestamp() {
for (int i = time.length(); --i >= 0; ) {
char c = time.charAt(i);
if ((c < '0' || c > '9') && (i != 0 || c != '-')) {
+ // Not a plain integer — treat as ISO 8601 and pass through
as-is.
return time;
}
}
+ // Plain integer: convert from seconds to ISO 8601, clamping to
EPOCH_MIN if needed.
try {
- return Instant.ofEpochSecond(Long.parseLong(time)).toString();
+ long seconds = Long.parseLong(time);
+ if (seconds < EPOCH_MIN) {
+ String instant = Instant.ofEpochSecond(seconds).toString();
+ String minValid = Instant.ofEpochSecond(EPOCH_MIN).toString();
+ log.warn("Output timestamp \""
+ + time
+ + "\" (resolved to "
+ + instant
+ + ") is before the minimum value accepted by the jar
tool ("
+ + minValid
+ + "). Clamping to minimum. "
+ + "If you use SOURCE_DATE_EPOCH=0, set it to at least "
+ + EPOCH_MIN
Review Comment:
⚠️ **Warning message uses raw epoch-seconds instead of the ISO string**
`"set it to at least " + EPOCH_MIN + ‘.’` emits `315532802` — an opaque
number users must mentally convert to a date. `minValid` (the human-readable
ISO string `1980-01-01T00:00:02Z`) is already on the stack two lines above. Use
it here:
```java
+ "If you use SOURCE_DATE_EPOCH=0, set it to at
least "
+ minValid
+ ‘.’);
```
This makes the warning directly actionable — users see `set it to at least
1980-01-01T00:00:02Z` and can copy-paste the value into their build tooling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
