-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
parsing of reproducible builds 'project.build.outputTimestamp' cannot handle default dateFormat #674
Comments
I could do an MR but I am unsure how to proceed.... IMHO the |
Mhh this is a weird one. The page https://maven.apache.org/guides/mini/guide-reproducible-builds.html mentions the following should be supported:
Which aligns with https://maven.apache.org/ref/3.3.1/maven-model-builder/ that states that the When clicking on the somewhat hidden link in (Enable Reproducible Builds mode for plugins, by adding project.build.outputTimestamp property to the project's pom.xml) mentioned in https://maven.apache.org/guides/mini/guide-reproducible-builds.html one comes across the following:
a For Maven 2.x and 3.x, the value can be defined as an equivalent property:
So I guess here is one source of this secret When now checking the code of the plugin: git-commit-id-maven-plugin/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java Lines 1484 to 1496 in fd67b7e
I see in the commentary that this was inspired by https://github.com/apache/maven-archiver/blob/a3103d99396cd8d3440b907ef932a33563225265/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L765. Here we have again the same/similar pattern of A more recent version got rid of this hard-coded string: So I would suggest to use a similar parsing mechanism and drop the pattern with
Of course I think it also makes sense to update the comments to mention an git-commit-id-maven-plugin/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java Line 1125 in fd67b7e
Does this make sense? |
I finally got some time to look into this again. Unfortunately your suggestion does not work as (also new to me) the new java date parsers are not able to parse the RFC 822 format?!
I then went ahead and modified the last lines of https://github.com/git-commit-id/git-commit-id-maven-plugin/blob/master/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java#L1464 to be the following (i.e. just re-try parsing it using the format the timestamp is created with from git);
this worked for this plugin, unfortunately then later on in my project the
I thought that maybe using the format Ultimately I am unsure how to proceed and will stick with the workaround I am currently configured (explicitly configuring a working format in the plugins configurion in pom.xml) |
The time must be formatted as ISO 8601. Can you check if the code in https://github.com/git-commit-id/git-commit-id-maven-plugin/pull/699/files would cover your case? |
Thank you very much for your response! Unfortunately it does not, as (maybe this was not clear enough in my ramblings above) the plugin itself generates an invalid dateformat for further maven processing (as seen here: https://github.com/git-commit-id/git-commit-id-maven-plugin/blob/master/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java#L513). I cobbled together a quick reproducer project: git-plugin-timestamp-problem-demo.zip
This is caused because I set the |
Thanks for the additional explanations and the reproducer! Sorry maybe it was just me being slow in understanding what you really mean, but I think I got it now. For that to happen (without any extra Likely this would mean it's a breaking change, since as a plugin developer I don't know what weird features the plugin users rely on... |
no worries :) As I said in my original message, doing So IMHO yes, you should do a breaking change as the "new" default is more sensible - this could clearly be advertised in the release notes and so on ("default format changed, if you need the old format add Also, like I said above, I did exactly this locally (i.e. changed line 513 in GitCommitIdMojo to be |
No worries, just to make sure I don't overlook something I ran a quick test and setting the I have tested #699 with the demo profile that is encoded in the pom:
With a configuration of:
this produces a |
It does not work on my machine :( I see that your changes should be good in theory but there's something wrong with it, I did the following, where is the mistake (could you try it on your environment):
I can see it in the plugins output that the wrong value is generated:
What am I missing/doing wrong?? Again thank you for your investigations and effort, I really appreciate it 👍 |
Mhhh.
That for me sounds like The support for reproducible builds (ISO8601) should have been added to the maven-jar-plugin in version 3.2.X (via https://issues.apache.org/jira/browse/MJAR-263) and you are using the latest available version (3.3.0). The documentation of the maven-jar-plugin (https://github.com/apache/maven-jar-plugin/blob/63f9c982d49f046519088490d7a8af5a7cd647fb/src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java#L144) even claims it can process '2019-10-05T20:37:42+06:00'. Maybe I'm going insane, but i think it would be worth to try your setup entirely WITHOUT the git-commit-id plugin. I'll try to troubleshoot this further with your reproducer too... |
I have stripped down your demo project to a really minimal thing. Project:
Congratulation, you found an issue in the maven-jar-plugin! :-) |
Ohhhhhh we produce the time as |
Could you perhaps also experiment why your java version reports the time as 2024-02-15T13:54:59+0100 (without the import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.TimeZone;
public class Main {
public static void main(String[] args) {
String dateFormat = "yyyy-MM-dd'T'HH:mm:ssXXX";
String dateFormatTimeZone = "GMT-01:00"; // can be null
SimpleDateFormat smf = new SimpleDateFormat(dateFormat);
if (dateFormatTimeZone != null) {
smf.setTimeZone(TimeZone.getTimeZone(dateFormatTimeZone));
}
System.out.println(smf.format(new Date()));
}
} As per https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html#iso8601timezone the Notes to myself: |
Read my answer in the JIRA issue. Straight away: |
thanks for your feedback, let' recapitulate (I am in Germany so I use my timezone as an example):
I would not have used the same words as @michael-o but yeah, using RFC822-format as default output format from the git-commit-id-plugin was probably not a good choice and should be changed in the next release to ISO8601 (see also my reasoning above and I am under the impression that you agree to that) I still have the issue that, when used, the updated plugin does not work as expected, I am really lost at this as I have no knowledge how maven is doing things:
that is exactly like I would expect it to be and how it should be
WTF is going on?! Where is he getting the wrong dateformat again from?? It must be set somewhere by some black Maven magic that I do know nothing of ;( I tried to debug it setting field watchpoint on |
Thanks everyone for your input. First of all, I agree the plugin should change to ISO8601-formatted dates. When running with
My theory or the black maven magic that makes your date formatted like a I guess this should be changed to |
Old formmat: yyyy-MM-dd'T'HH:mm:ssZ, RFC 822 New format: yyyy-MM-dd'T'HH:mm:ssXXX ISO 8601 Related to git-commit-id/git-commit-id-maven-plugin#674. This change is required to make the times produced by the git-commit-id-maven-plugin usable for Maven's reproducible builds, see https://maven.apache.org/guides/mini/guide-reproducible-builds.html. Timestamp for reproducible output archive entries must either formatted as ISO 8601 or as an int representing seconds since the epoch. Example usage might be <properties> <project.build.outputTimestamp>${git.commit.time}</project.build.outputTimestamp> </properties> See gh-39606
#674: change timeformat from RFC822 to ISO 8601 to support maven's reproducible build feature
Thanks again for the report and your patience with me. The sprint boot has updated the default format to So thanks again, I'm going ahead and close the ticket now :-) |
Describe the bug (required)
in https://maven.apache.org/guides/mini/guide-reproducible-builds.html it is mentioned that you could do
<project.build.outputTimestamp>${git.commit.time}</project.build.outputTimestamp>
and this is IMHO indeed a neat idea.If you do so, however, you'll get
This is because the defaullt
dateFormat
isyyyy-MM-dd'T'HH:mm:ssZ
butparseOutputTimestamp
only tries to parse the formatyyyy-MM-dd'T'HH:mm:ssXXX
It can be work-arounded by specifying the configuration
<dateFormat>yyyy-MM-dd'T'HH:mm:ssXXX</dateFormat>
but this shouldnt be neccessary.Tell us about your plugin configuration (required)
Tell us about the Plugin version used (required)
6.0.0
Tell us about the Maven version used (required)
Steps to Reproduce (required)
just configure the property
<project.build.outputTimestamp>${git.commit.time}</project.build.outputTimestamp>
in the POMAre there any stacktraces or any error messages? (required)
Is there a (public) project where this issue can be reproduced? (optional)
No response
Your Environment (optional)
No response
Context (optional)
No response
The text was updated successfully, but these errors were encountered: