RepackageMojo doesn't support 1 digit numerical values for project.build.outputTimestamp by nielsbasjes · Pull Request #37438 · spring-projects/spring-boot

nielsbasjes · 2023-09-17T11:59:43Z

For a build to be reproducible the project.build.outputTimestamp needs to be set to a valid value for the spring-boot-maven-plugin.

This can be either a number (indicating the epoch seconds) or an ISO 8601 timestamp.

These example values all work as expected (plugin version 3.1.3):

'2023-09-17T12:34:56Z' --> Valid timestamp
'1694946896' --> Valid timestamp
'a' --> Valid (= null, no timestamp)
'' --> Valid (= null, no timestamp)
'aa' --> Invalid (parse error, build fails)

This merge request fixes the edge case of having a single character numerical timestamp which has been implemented differently from what Maven does.

Maven accepts a '0' as a valid timestamp.
See for example https://github.com/apache/maven-archiver/blob/master/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L762

The Spring Boot maven plugin currently sees the '0' as a null instead (i.e. no timestamp set instead of epoch=0).

…utTimestamp

vy · 2023-09-18T06:53:18Z

I can confirm @nielsbasjes finding that setting project.build.outputTimestamp to 0 causes spring-boot:repackage to generate a different artifact (i.e., not reproducible) at each run, whereas the rest of the Maven ecosystem [supporting reproducible builds] works just fine.

nielsbasjes · 2023-09-18T09:06:02Z

I went through the build logs to see why it failed.

I found this.
Please assist in helping me understand how (and if) I can fix this problem.
At this time I do not see the relation between my changes and this failure.

Caused by: java.nio.file.AccessDeniedException: /tmp/junit-image-cache-wkkvnpipvv-build/committed/io.buildpacks.lifecycle.cache.metadata
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:90)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystemProvider.implDelete(UnixFileSystemProvider.java:248)
	at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
	at java.base/java.nio.file.Files.delete(Files.java:1152)
	at org.springframework.util.FileSystemUtils$1.visitFile(FileSystemUtils.java:90)
	at org.springframework.util.FileSystemUtils$1.visitFile(FileSystemUtils.java:87)
	at java.base/java.nio.file.Files.walkFileTree(Files.java:2811)
	at java.base/java.nio.file.Files.walkFileTree(Files.java:2882)
	at org.springframework.util.FileSystemUtils.deleteRecursively(FileSystemUtils.java:87)
	at org.springframework.boot.maven.BuildImageTests.lambda$whenBuildImageIsInvokedWithBindCaches$19(BuildImageTests.java:422)
	at org.springframework.boot.maven.MavenBuild.execute(MavenBuild.java:190)
	... 124 more

wilkinsona · 2023-09-18T09:37:40Z

@nielsbasjes there's no connection. It's an unfortunate intermittent failure that we, as far as I can recall, have only seen on GitHub actions. Please ignore it for now. We will build your changes locally prior to merging them.

hgschmie · 2023-09-19T04:46:03Z

...ols/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/RepackageMojo.java

-		// Maven ignores a single-character timestamp as it is "useful to override a full
-		// value during pom inheritance"
-		if (this.outputTimestamp == null || this.outputTimestamp.length() < 2) {
+		// Maven ignores a single-character non-digit timestamp as it is


It feels to me that the right fix would be to declare a dependency on the maven-archiver artifact and call MavenArchiver#parseBuildOutputTimestamp(String outputTimestamp) to get the exact same behavior (and then parse the result into a FileTime object). This is still incomplete and may break in other ways.

See https://github.com/nielsbasjes/maven-archiver/blob/master/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L768C37-L801 for the actual implementation.

Thanks. We'll polish the PR when we merge it and then use the same code as your linked Mojo.

mhalbritter · 2023-09-21T14:52:00Z

When merging this, polish the contribution so that the parsing of the timestamp matches the implementation in https://github.com/nielsbasjes/maven-archiver/blob/master/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L768C37-L801

A dependency for just one method feels unjustified.

nielsbasjes · 2023-09-21T16:06:28Z

@mhalbritter Would you like me to update the pull request in this way? Or are you going to pick this up?

hgschmie · 2023-09-21T16:11:50Z

When merging this, polish the contribution so that the parsing of the timestamp matches the implementation in https://github.com/nielsbasjes/maven-archiver/blob/master/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L768C37-L801

A dependency for just one method feels unjustified.

then you just set yourself up for another breakage some time in the future. The maven-archiver artifact is available anyway, declaring a dependency is not a big deal and it allows you to write less code but call code that is maintained by the maven project.

Not sure why you consider this unjustified. Anyway, your call.

mhalbritter · 2023-09-21T16:28:47Z

@mhalbritter Would you like me to update the pull request in this way? Or are you going to pick this up?

If you like, you can do it, i'd be happy. If you don't find the time, I'll do it on the merge.

nielsbasjes · 2023-09-21T18:29:15Z

Working on it.

…ild.outputTimestamp

nielsbasjes · 2023-09-21T18:45:44Z

I have now copied part of the maven archiver code as I understood this is what you wanted (instead of adding the maven-archiver dependency).
Correct?

mhalbritter · 2023-09-22T09:22:19Z

Thanks a lot!

See gh-37438

…tamp" See gh-37438

fix: Correctly handle 1 digit numerical values for project.build.outp…

35b928e

…utTimestamp

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 17, 2023

mhalbritter changed the title ~~fix: Correctly handle 1 digit numerical values for project.build.outputTimestamp~~ RepackageMojo doesn't support 1 digit numerical values for project.build.outputTimestamp Sep 18, 2023

mhalbritter added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 18, 2023

mhalbritter added this to the 2.7.x milestone Sep 18, 2023

hgschmie reviewed Sep 19, 2023

View reviewed changes

mhalbritter added the for: merge-with-amendments Needs some changes when we merge label Sep 21, 2023

fix: Use the Maven Archiver implementation for parsing the project.bu…

f6b7b3f

…ild.outputTimestamp

mhalbritter self-assigned this Sep 22, 2023

mhalbritter mentioned this pull request Sep 22, 2023

RepackageMojo doesn't support 1 digit numerical values for project.build.outputTimestamp #37534

Closed

mhalbritter modified the milestones: 2.7.x, 2.7.17 Sep 22, 2023

mhalbritter pushed a commit that referenced this pull request Sep 22, 2023

Use the Maven Archiver implementation for parsing outputTimestamp

a1b08e1

See gh-37438

mhalbritter added a commit that referenced this pull request Sep 22, 2023

Polish "Use the Maven Archiver implementation for parsing outputTimes…

a3611ae

…tamp" See gh-37438

mhalbritter closed this in f623012 Sep 22, 2023

nielsbasjes deleted the RepackageBuildOutputTimestamp branch September 22, 2023 19:08

Netyyyy mentioned this pull request Oct 20, 2023

External dependencies upgrade - Spring Boot 2.7.17 and Spring Cloud 2021.0.8 Azure/azure-sdk-for-java#37261

Closed

Netyyyy mentioned this pull request Oct 27, 2023

External dependencies upgrade - Spring Boot 2.7.17 and Spring Cloud 2021.0.8 Azure/azure-sdk-for-java#37420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RepackageMojo doesn't support 1 digit numerical values for project.build.outputTimestamp#37438

RepackageMojo doesn't support 1 digit numerical values for project.build.outputTimestamp#37438
nielsbasjes wants to merge 2 commits intospring-projects:mainfrom
nielsbasjes:RepackageBuildOutputTimestamp

nielsbasjes commented Sep 17, 2023

Uh oh!

vy commented Sep 18, 2023

Uh oh!

nielsbasjes commented Sep 18, 2023

Uh oh!

wilkinsona commented Sep 18, 2023

Uh oh!

hgschmie Sep 19, 2023

Uh oh!

mhalbritter Sep 21, 2023

Uh oh!

mhalbritter commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

hgschmie commented Sep 21, 2023

Uh oh!

mhalbritter commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

mhalbritter commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

nielsbasjes commented Sep 17, 2023

Uh oh!

vy commented Sep 18, 2023

Uh oh!

nielsbasjes commented Sep 18, 2023

Uh oh!

wilkinsona commented Sep 18, 2023

Uh oh!

hgschmie Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

mhalbritter Sep 21, 2023

Choose a reason for hiding this comment

Uh oh!

mhalbritter commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

hgschmie commented Sep 21, 2023

Uh oh!

mhalbritter commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

nielsbasjes commented Sep 21, 2023

Uh oh!

mhalbritter commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants