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

Travis configuration improvements #643

Merged
merged 1 commit into from
Mar 12, 2019

Conversation

wborn
Copy link
Member

@wborn wborn commented Mar 10, 2019

This PR makes it possible again to follow progress of a Travis build while it's running. While building it shows the reactor progress information while making sure the build doesn't timeout by appending zero width characters every 3 minutes.

While I was at it I've made some other configuration improvements as well:

  • Uses the new Xenial Travis images which easily allows for running Java 11 builds
  • Builds using OpenJDK 8 instead of Oracle JDK 8. Oracle JDK 8 is no longer available in the Xenial images. I've done a lot of builds already with it and it works equally well.
  • Adds a new OpenJDK 11 build which is allowed to fail. This makes it easy to see what needs to be done for successfully compiling the code with Java 11. It disables the enforcer for the Java 11 build.
  • On successful builds only the reactor summary is printed (instead of the most recent 200 lines)
  • Removes JVM options that are no longer supported in Java 8 and which result in warnings:
    -XX:PermSize=512m -XX:MaxPermSize=1g
  • Removes the unused Paper UI NPM cache from the build

I've also tested a similar configuration with the openhab2-addons build (see this wborn/openhab2-addons build) where the timeout would occur during feature validation. The build times remain the same with this configuration.

Signed-off-by: Wouter Born <[email protected]>
Copy link
Contributor

@maggu2810 maggu2810 left a comment

Choose a reason for hiding this comment

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

I am fine with the changes in general.

I can't believe that the build time is the same if we are building for 8 + 11 now instead of 8 only.
How is it working? Are the Xenial VMs more powerful?

Does it make sense to enable a build for 11 as long as we are using Xtext <2.17?
Can it ever succeed without bumping Xtext?

@wborn
Copy link
Member Author

wborn commented Mar 11, 2019

How is it working? Are the Xenial VMs more powerful?

It uses a build matrix so Travis typically builds these jobs in parallel. We use an even bigger build matrix for openhab-docker where Travis always executes 3 jobs in parallel. The Java 11 job currently always fails before the Java 8 build succeeds so it shouldn't increase the Travis (wall clock) build time.

Does it make sense to enable a build for 11 as long as we are using Xtext <2.17?

I think it does because it makes it easy to see what other issues there are. E.g. it now fails because of some Mockito issue that needs further investigation. Another reason for adding a Java 11 build is that it shows that we are actively working on supporting newer Java versions which is food for community topics. It also helps to keep the discussion going so we actually get there (like SAT warnings nag people to fix them 😉).

I wasn't aware that Xtext needs to be updated for supporting Java 11. It wasn't mentioned in eclipse-archived/smarthome#4369. Apparently Xtext 2.17 has only been released last week (announcement). Do you think we can easily upgrade Xtext from 2.14 to 2.17? The upgrade should also fix an issue with the xtext-maven-plugin when running a parallel maven build (see eclipse/xtext-maven#37 by @pfink).

@maggu2810
Copy link
Contributor

@wborn I don't know at which placed I reported it. Just found this one: openhab/static-code-analysis#325 (comment)

@cweitkamp
Copy link
Contributor

Do not forget to edit the version constraint in the pom.xml:

[WARNING] Rule 0: org.apache.maven.plugins.enforcer.RequireJavaVersion failed with message:
Detected JDK Version: 11.0.2 is not in the allowed range [1.8.0-40,1.9).

@wborn
Copy link
Member Author

wborn commented Mar 12, 2019

I'd prefer to only do that when we know the code successfully builds using JDK 11. That way it's immediately clear for developers what JDK needs to be installed for a successful build. In the Travis config I've disabled the enforcer with extra_maven_opts only for builds that aren't using openjdk8.

@maggu2810
Copy link
Contributor

@wborn To skip something, so changing the default behaviour should be only done if really wanted.
Wouldn't it make more sense to skip the enforcer plugin if the used JDK is the one we know to skip it (openjdk11) instead of skip it if it is not "openjdk8"?
Sure, it should not matter, just thinking about it.

@maggu2810
Copy link
Contributor

Do you think we can easily upgrade Xtext from 2.14 to 2.17?

I could never get used to it.
Perhaps someone would like to give it a try (we could track it in a separate issue).

Anything missing in this PR or ready to be merged?

@wborn
Copy link
Member Author

wborn commented Mar 12, 2019

It's ready to be merged. :-)

@wborn
Copy link
Member Author

wborn commented Mar 12, 2019

Perhaps someone would like to give it a try (we could track it in a separate issue).

When time permits I can give it a try. There's now #646 for this.

Copy link
Contributor

@maggu2810 maggu2810 left a comment

Choose a reason for hiding this comment

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

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants