[
https://issues.apache.org/jira/browse/MSHADE-471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17836993#comment-17836993
]
ASF GitHub Bot commented on MSHADE-471:
---------------------------------------
rmannibucau commented on PR #220:
URL:
https://github.com/apache/maven-shade-plugin/pull/220#issuecomment-2054146138
@hboutemy you merged but I didn't see any test showing that it helps, can
you try to work on it - not asking for an IT, no worries.
For example a trivial test has the same bug than before the fix so wonder if
we should revert the code change and work on a better fix or just let it like
that:
```
@Test
public void testDST() throws IOException, MojoExecutionException {
final ZoneOffset entryOffset = ZoneOffset.ofHours(2);
final Instant entryInstant = LocalDateTime.of(2024, Month.APRIL, 14,
20, 0).toInstant(entryOffset);
final Instant entryExtraInstant = LocalDateTime.of(2024,
Month.APRIL, 14, 20, 1).toInstant(entryOffset);
final File input = tmp.newFile("in.jar");
try (final JarOutputStream jar = new
JarOutputStream(Files.newOutputStream(input.toPath()))) {
final X5455_ExtendedTimestamp x5455 = new
X5455_ExtendedTimestamp();
x5455.setCreateFileTime(FileTime.from(entryExtraInstant));
final JarEntry entry = new JarEntry("test.txt");
entry.setTime(entryInstant.toEpochMilli());
entry.setExtra(ExtraFieldUtils.mergeLocalFileDataData(new
ZipExtraField[]{ x5455 }));
jar.putNextEntry(entry);
jar.closeEntry();
}
final File output = new File(tmp.getRoot(), "output.jar");
final ShadeRequest request = new ShadeRequest();
request.setJars(singleton(input));
request.setUberJar(output);
request.setResourceTransformers(emptyList());
request.setRelocators(emptyList());
request.setFilters(emptyList());
request.setShadeSourcesContent(false);
final DefaultShader shader = new DefaultShader();
shader.shade(request);
final Instant outInstant;
try (final JarFile out = new JarFile(output)) {
outInstant =
out.getJarEntry("test.txt").getLastModifiedTime().toInstant();
}
assertEquals(entryExtraInstant, outInstant);
}
```
will fail with:
```
Expected :2024-04-14T18:01:00Z
Actual :2024-04-14T16:00:00Z
```
So hour is wrong (offset management corrupted the value since it is not from
a jar we manage but a jar we consume?) and time is likely wrong too (wrong time
is read).
Main issue for me is that this fix seems to mix dates (entry time and extra
field time - times in practise) so not really sure it is reliable.
However it highlights the fact we don't propagate extra fields which can be
another trivial issue to handle (just mentionning to enforce even more the date
issue, not asking for a fix for this ticket).
In terms of fix, what do you intend to do: use the entry time so ignore the
x5455 field? use the x5455 create time? use the x5455 modify time? ...
What is wrong using the `Date` timestamp? (`((X5455_ExtendedTimestamp)
field).getCreateJavaTime()` for ex), how does it affect negatively reproducible
builds since it is a timestamp related to the UTC "0" (1970)?
About building in UTC I think I would just enable to set default `ZoneId`
since `java.util.zip.ZipUtils#javaEpochToLocalDateTime` uses
`ZoneId.systemDefault()` so it is mainly a matter of calling
`TimeZone.setDefault` before the build and after the build.
I was more thinking to do it with java system properties but ultimately we
could allow to do it in the mojo too and ultimately enable maven to do it per
mojo (needs a javaagent to modify the jvm default impl to make it contextual
but this is not a big deal, @gnodet already did it for mvnd and technically
maven would be the same kind of trick - even easier actually there.
So long story short: there is a date selection/handling to fix, the timezone
issue is likely not one and we still have a workaround to not need it (doc
point).
Side note: this extra field is only valid until 2037 so it can be sane to
just ignore it and use the entry time as this since we don't know the original
offset and we can't invent it until we have the extra field which is known as
UTC relative.
> still timestamp issues
> ----------------------
>
> Key: MSHADE-471
> URL: https://issues.apache.org/jira/browse/MSHADE-471
> Project: Maven Shade Plugin
> Issue Type: Bug
> Affects Versions: 3.5.2
> Reporter: Herve Boutemy
> Assignee: Herve Boutemy
> Priority: Major
> Fix For: 3.5.3
>
>
> MSHADE-420 is incomplete, problems still appear like
> {noformat}1 / 1
> target/reference/eu.maveniverse.maven.plugins/toolbox-0.1.5-cli.jar
> toolbox/target/toolbox-0.1.5-cli.jar
> --- target/reference/eu.maveniverse.maven.plugins/toolbox-0.1.5-cli.jar
> +++ toolbox/target/toolbox-0.1.5-cli.jar
> ├── zipinfo {}
> │ @@ -735,96 +735,96 @@
> │ -rw---- 2.0 fat 50 bl defN 24-Jan-23 12:20
> META-INF/maven/org.jline/jline/pom.properties
> │ --rw---- 2.0 fat 0 bl defN 23-Oct-12 07:38
> META-INF/native-image/jansi/
> │ --rw---- 2.0 fat 12491 bl defN 23-Oct-12 07:38
> META-INF/native-image/jansi/jni-config.json
> ...
> │ --rw---- 2.0 fat 17329 bl defN 23-Oct-12 07:38
> META-INF/maven/org.fusesource.jansi/jansi/pom.xml
> │ --rw---- 2.0 fat 60 bl defN 23-Oct-12 07:38
> META-INF/maven/org.fusesource.jansi/jansi/pom.properties
> │ +-rw---- 2.0 fat 0 bl defN 23-Oct-12 06:38
> META-INF/native-image/jansi/
> │ +-rw---- 2.0 fat 12491 bl defN 23-Oct-12 06:38
> META-INF/native-image/jansi/jni-config.json
> ...
> │ +-rw---- 2.0 fat 17329 bl defN 23-Oct-12 06:38
> META-INF/maven/org.fusesource.jansi/jansi/pom.xml
> │ +-rw---- 2.0 fat 60 bl defN 23-Oct-12 06:38
> META-INF/maven/org.fusesource.jansi/jansi/pom.properties
> │ -rw---- 2.0 fat 0 bl defN 24-Apr-11 20:18
> eu/maveniverse/maven/toolbox/shared/
> {noformat}
> https://github.com/jvm-repo-rebuild/reproducible-central/blob/master/content/eu/maveniverse/maven/toolbox/README.md
> (fixed since then by using a TZ, but initial UTC shows the issue)
> looks related to DST: probably need to apply what has been done in
> plexus-archiver
> https://github.com/codehaus-plexus/plexus-archiver/commit/b9ea3bf0e4c25c0a5cf1bcbc76e691067003dc36
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
