Skip to content
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

Build fixes #15

Merged
merged 3 commits into from Apr 29, 2020
Merged

Build fixes #15

merged 3 commits into from Apr 29, 2020

Conversation

stenlarsson
Copy link
Contributor

@stenlarsson stenlarsson commented Apr 22, 2020

This fixes a number of issues with the build system. I actually implemented these back in February, but didn't get around to creating a PR until now.

Automatically set driver version from artifact version

The version of the driver was hard coded in AthenaDatabaseMetaData which causes the tests to fail when bumping the version during the release process. To fix this we create a properties file, and tell Maven to write the project version to this file. Then we load and parse the version number from this properties file.

I have tried various other solutions that involve replacing the version in the code directly, but none of them works when running the unit tests.

Rename release profile

For some reason the "release" profile is always active, which causes it to try to sign the jar file even when doing a simple mvn install. Not sure what activates it, but renaming it seems to prevent it from happening.

Only specify HTML5 javadoc in version 1.9 or higher

When building the jar in Java 8 it fails because the -html5 flag is not supported.

Move to next snapshot release

This should have been done after the last release. Should not be done as part of this PR.

@stenlarsson stenlarsson self-assigned this Apr 22, 2020
Copy link
Contributor

@grddev grddev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also had problems building, so it is great to see improvements.

That being said, It feels like the version-number thing is very complex, and something basically every project would need to solve. Is there really no better way to handle this?

driverMajorVersion = Integer.parseInt(matcher.group(1));
driverMinorVersion = Integer.parseInt(matcher.group(2));
} else {
throw new RuntimeException("Could not parse driver version: " + driverVersion);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you throw a runtime exception from a static block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will get the error when you try to check the version:

Exception in thread "main" java.lang.ExceptionInInitializerError
	at io.burt.athena.AthenaDatabaseMetaData.getDriverVersion(AthenaDatabaseMetaData.java:42)
	at io.burt.athena.Test.main(Test.java:16)
Caused by: java.lang.RuntimeException: Could not parse driver version: 0.3a.0-SNAPSHOT
	at io.burt.athena.AthenaDriverInfo.<clinit>(AthenaDriverInfo.java:27)
	... 2 more

@stenlarsson
Copy link
Contributor Author

That being said, It feels like the version-number thing is very complex, and something basically every project would need to solve. Is there really no better way to handle this?

The solution I have implemented here is based on the top answer to this StackOverflow post: https://stackoverflow.com/questions/3697449/retrieve-version-from-maven-pom-xml-in-code

pom.xml Outdated
@@ -5,7 +5,7 @@

<groupId>io.burt</groupId>
<artifactId>athena-jdbc</artifactId>
<version>0.2.0</version>
<version>0.3.0-SNAPSHOT</version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you need to modify this to make sure the changes work, but in general, only change project versions on master or in release-only branches. The reason is that the current version may be 0.4 or 9.1 by the time this gets merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, reverted.

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class AthenaDriverInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a package private class, there isn't any need to expose it unless we have to. Someone might start using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

The version of the driver was hard coded in AthenaDatabaseMetaData which causes the tests to fail when bumping the version during the release process. To fix this we create a properties file, and tell Maven to write the project version to this file. Then we load and parse the version number from this properties file.

There are also another properties file `META-INF/maven/io.burt/athena-jdbc/pom.properties` added automatically to the jar file, but this file cannot be used in the unit tests.
For some reason the "release" profile is always active, which causes it to try to sign the jar file even when doing a simple "mvn install". Not sure what activates it, but renaming it seems to prevent it from happening.
When building the jar in Java 8 it fails because the -html5 flag is not supported.
@stenlarsson stenlarsson merged commit cc917ef into master Apr 29, 2020
@stenlarsson stenlarsson deleted the build-fixes branch April 29, 2020 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants