gnodet-bot commented on code in PR #401:
URL: https://github.com/apache/maven-archiver/pull/401#discussion_r4108669438


##########
src/main/java/org/apache/maven/shared/archiver/MavenArchiver.java:
##########
@@ -69,8 +71,18 @@
  */
 public class MavenArchiver {
 
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(MavenArchiver.class);
+
     private static final String CREATED_BY = "Maven Archiver";
 
+    /**
+     * Minimum timestamp value for ZIP/JAR entries (1980-01-01T00:00:02Z).
+     * Timestamps before this value are clamped to it with a warning.
+     *
+     * @since 3.7.1

Review Comment:
   ⚠️ **Wrong version tag** — master is `4.0.0-beta-6-SNAPSHOT` (last 3.x 
release was 3.6.6, no 3.7.x line exists).
   
   ```suggestion
        * @since 4.0.0-beta-6
   ```



##########
src/main/java/org/apache/maven/shared/archiver/MavenArchiver.java:
##########
@@ -701,16 +713,32 @@ public static Optional<Instant> 
parseBuildOutputTimestamp(String outputTimestamp
 
         // Number representing seconds since the epoch
         if (isNumeric(outputTimestamp)) {
-            final Instant date = 
Instant.ofEpochSecond(Long.parseLong(outputTimestamp));
+            Instant date = 
Instant.ofEpochSecond(Long.parseLong(outputTimestamp));
+            if (date.isBefore(DATE_MIN)) {
+                LOGGER.warn(
+                        "Timestamp '{}' is before the minimum date for ZIP/JAR 
entries."
+                                + " Clamping to DATE_MIN ({}). See 
https://github.com/apache/maven-jar-plugin/issues/595";,
+                        date,
+                        DATE_MIN);
+                date = DATE_MIN;
+            }

Review Comment:
   💡 **DRY** — this 7-line clamping block is duplicated verbatim in the 
ISO-8601 branch below (lines 734-741). Extract to a helper:
   
   ```suggestion
               date = clampToDateMin(date, outputTimestamp);
   ```
   
   With a private helper:
   ```java
   private static Instant clampToDateMin(Instant date, String originalInput) {
       if (date.isBefore(DATE_MIN)) {
           LOGGER.warn(
                   "Timestamp '{}' (parsed from '{}') is before the minimum 
date for ZIP/JAR entries."
                           + " Clamping to DATE_MIN ({}). See 
https://github.com/apache/maven-jar-plugin/issues/595";,
                   date, originalInput, DATE_MIN);
           return DATE_MIN;
       }
       return date;
   }
   ```
   
   This also fixes a minor UX issue: the current warning logs the parsed 
`Instant` but not the original input string — a user who set 
`SOURCE_DATE_EPOCH=0` sees `1970-01-01T00:00:00Z` in the warning, which doesn't 
immediately connect to their `0` input.



-- 
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]

Reply via email to