Cli add version argument #376
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://github.com/onthegomap/planetiler/actions/runs/3553600311 ℹ️ Base Logs 3a434a4ℹ️ This Branch Logs 293ab4a |
msbarry
reviewed
Nov 11, 2022
planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java
Outdated
Show resolved
Hide resolved
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> |
There was a problem hiding this comment.
I think it would be useful to print the git hash and build time as well as version. It looks like this approach would give us that: https://jdonkervliet.com/2021/10/22/git-version-in-jar.html - you'd just have to add the buildnumber-maven-plugin, and configure maven-jar-plugin to add the Implementation-Build and Implementation-Timestamp properties. I tried it locally and got:
./mvnw -DskipTests clean package && unzip -p planetiler-dist/target/planetiler-dist-0.6-SNAPSHOT.jar META-INF/MANIFEST.MF
...
Implementation-Title: planetiler-dist
Implementation-Version: 0.6-SNAPSHOT
Implementation-Vendor-Id: com.onthegomap.planetiler
Implementation-Build: bd9f3ab22258da4eb9e6a5b0cbefd00fb0ff6df9
Implementation-Timestamp: 2022-11-11T11:11:20Z
Then we'd just have to figure out how to get those properties out of MANIFEST.MF.
There was a problem hiding this comment.
Which POMs did you add the plugin to to get it to add stuff to manifest?
This should allow getting info from manifest https://stackoverflow.com/a/3777116
There was a problem hiding this comment.
Great! That snippet looks like it should do it. I was able to get those in the manifest by adding to the root pom.xml build/plugins section:
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>buildnumber-maven-plugin</artifactId>
<version>3.0.0</version>
<executions>
<execution>
<phase>validate</phase>
<goals>
<goal>create</goal>
</goals>
</execution>
</executions>
</plugin>and also:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<archive>
<manifest>
<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
</manifest>
<manifestEntries>
<Implementation-Build>${buildNumber}</Implementation-Build>
<Implementation-Timestamp>${maven.build.timestamp}</Implementation-Timestamp>
</manifestEntries>
</archive>
</configuration>
</plugin>There was a problem hiding this comment.
I see. I checked and adding those two plugins makes build add version info to JAR but only to the smaller JAR without dependencies. Fat JAR with dependencies doesn't have that info.
Is that expected?
There was a problem hiding this comment.
Also how do you run the no-deps file?
I'm running:
$ java -cp planetiler-dist/target/planetiler-dist-0.6-SNAPSHOT.jar com.onthegomap.planetiler.Main --version
and get:
Exception in thread "main" java.lang.NoClassDefFoundError: org/openmaptiles/OpenMapTilesMain
Running the jar with dependencies work but since version is not in manifest it just prints null values.
There was a problem hiding this comment.
Good point, this approach looks problematic because the jars get built so many different ways (jib, assembly plugin, regular maven). Here's another approach that appears to work: c10f08d - basically it just builds a resource file in planetiler-core that captures the build info from maven and that goes everywhere the class files go so we don't need to worry about adding it to all the manifests. What do you think?
There was a problem hiding this comment.
Also how do you run the no-deps file
you need to add all of the transitive dependencies to the classpath as well. It gets a little tricky to find them all, something like `java -cp "planetiler-dist/target/planetiler-dist-0.6-SNAPSHOT.jar:planetiler-core/target/planetiler-core-0.6-SNAPSHOT.jar:..." com.onthegomap.planetiler.Main --version
There was a problem hiding this comment.
What do you think?
Looks better than the previous solution :)
… after version has been printed
|
Fixes #326 |
|
Looks great, thanks @ttomasz ! |
|
Kudos, SonarCloud Quality Gate passed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I saw #326 and even though I have no experience with Java I thought it would be easy enough to add.
Let me know if this approach is ok or not.
Running
$ java -jar planetiler-dist/target/planetiler-dist-*-with-deps.jar --versionresults in logger printing arguments and versions of modules at the end: