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

Upgrade build to use Gradle 6.4 #51

Merged
merged 25 commits into from
Jun 6, 2021
Merged

Conversation

kpitt
Copy link
Contributor

@kpitt kpitt commented Jun 4, 2021

I recently started working on converting a rather large legacy build from Ant to Gradle, and I was in need of a few updates to the JavaCC plugin. The first step was to get the plugin build running on a more modern version of Gradle. I saw that this has been an outstanding item for this plugin for awhile, so I thought I would contribute my work in the hopes of maybe helping to get things moving forward again.

I saw that there was an existing release-3.0.0 branch, but I did not base my changes directly off that branch because those commits would have become part of the pull request against master. I did, however, cherry pick several of the changes and I believe I have covered all of the other updates in one way or another.

All of the unit and acceptance tests are passing when building with Java 8 and Gradle 6.4. I set the automated tests to run against Gradle 6.0.1 to try to maximize compatibility. I wasn't able to do a full test of all of the publishing tasks for obvious reasons, but I was able to publish successfully to the local Maven cache.

kpitt and others added 24 commits June 4, 2021 10:57
Note that this requires Gradle 6.4 for building the plugin, but the
compiled plugin should still be compatible with Gradle 6.0 or higher.
These tests are best implemented as integration tests.
- The old `compile` and `runtime` configurations have been deprecated
  for quite some time, and have now been removed entirely in Gradle 7.0,
  so they need to be replaced with the appropriate `compileOnly` and
  `implementation` configurations.
- The `force` option for dependency resolution has been deprecated and
  replaced with the `strictly` version declaration.
A `settings.gradle` file is now required for every Gradle build, even
if it only has a single project.
The '/' character is no longer allowed in a project name.  Subprojects
are still defined by the subdirectory structure, but the names should
always be separated by ':' characters when referring to the subproject
names.
Gradle now returns an outcome of `NO_SOURCE` instead of `UP_TO_DATE`
when there are no source files to process.
The use of the `version` property has been deprecated for all tasks
that derive from `AbstractArchiveTask`.  It has been replaced by the
`archiveVersion` property to make it distinct from the project `version`
property.
The deprecation warnings for the `version` property do not cause the
acceptance tests to fail in Gradle 7, but changing these properties now
will prevent future failures when the `version` property is removed in
Gradle 8.
The `withSourcesJar()` and `withJavadocJar()` helper functions provided
by the `java` extension make it simpler to generate the extra sources
and Javadoc JAR artifacts without the need to create and maintain
custom tasks.
- Move `SuppressionCommentFilter` module under `Checker` module (moved
  in v8.1)
- Remove `FileContentsHolder` module (removed in v8.2)
- Remove `maxLineLength` property from `LeftCurly` check (removed in v8.2)
- Move `LineLength` check directly under `Checker` module (moved in v8.24)
The plugin is now built for a minimum of Java 1.8, so update the test
projects to reflect the same Java version.  This doesn't really change
the behavior of the tests, but just keeps everything consistent.
The plugin should be usable with any 6.x version, but Gradle 6.4 is
required to build the plugin, so force the acceptance tests to run
against Gradle 6.0.1 instead of defaulting to the version that is
running the build.
@zosrothko
Copy link
Member

Hi, welcome to the JavaCC project and thanks for your contribution. There is a check failure on Travis CI. Could you check it and maybe fix it before I merge your pull request?

@kpitt
Copy link
Contributor Author

kpitt commented Jun 5, 2021

Looks like Travis CI was unable to install the JDK. The error message gives the impression that they no longer support Java 8.

Installing oraclejdk8
$ export JAVA_HOME=~/oraclejdk8
$ export PATH="$JAVA_HOME/bin:$PATH"
$ ~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"
Ignoring license option: BCL -- using GPLv2+CE by default
install-jdk.sh 2020-06-02
Expected feature release number in range of 9 to 17, but got: 8
The command "~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"" failed and exited with 3 during .

I'll have to take a look at the Travis config and see what the options are. Unfortunately, it could be problematic if they no longer allow Java 8. The plugin tests use Powermock in a way that isn't compatible with the module access restrictions in Java 9+, and I already wasted quite a few hours trying to get them to work with Java 11 before giving up and falling back to Java 8. It certainly isn't the preferred mechanism for testing and it would be nice to eliminate that dependency, but I don't have a good feel right now for what it would take to accomplish the same results without Powermock.

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@48917fa). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #51   +/-   ##
=========================================
  Coverage          ?   87.73%           
  Complexity        ?      149           
=========================================
  Files             ?       23           
  Lines             ?      432           
  Branches          ?       28           
=========================================
  Hits              ?      379           
  Misses            ?       49           
  Partials          ?        4           
Impacted Files Coverage Δ
...gins/javacc/compiler/JavaccSourceFileCompiler.java 100.00% <ø> (ø)
.../javacc/programexecution/JavaccExecutorAction.java 100.00% <ø> (ø)
...s/javacc/programexecution/JjdocExecutorAction.java 100.00% <ø> (ø)
.../javacc/programexecution/JjtreeExecutorAction.java 100.00% <ø> (ø)
...ca/coglinc/gradle/plugins/javacc/JavaccPlugin.java 93.93% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48917fa...079e931. Read the comment docs.

@kpitt
Copy link
Contributor Author

kpitt commented Jun 5, 2021

I changed the config to use OpenJDK 8 instead of the Oracle JDK, and it looks like everything passed this time.

@zosrothko
Copy link
Member

Looks like Travis CI was unable to install the JDK. The error message gives the impression that they no longer support Java 8.

Installing oraclejdk8
$ export JAVA_HOME=~/oraclejdk8
$ export PATH="$JAVA_HOME/bin:$PATH"
$ ~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"
Ignoring license option: BCL -- using GPLv2+CE by default
install-jdk.sh 2020-06-02
Expected feature release number in range of 9 to 17, but got: 8
The command "~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"" failed and exited with 3 during .

I'll have to take a look at the Travis config and see what the options are. Unfortunately, it could be problematic if they no longer allow Java 8. The plugin tests use Powermock in a way that isn't compatible with the module access restrictions in Java 9+, and I already wasted quite a few hours trying to get them to work with Java 11 before giving up and falling back to Java 8. It certainly isn't the preferred mechanism for testing and it would be nice to eliminate that dependency, but I don't have a good feel right now for what it would take to accomplish the same results without Powermock.

OK no problem to let it as it is as soon the Travis build is ok

@zosrothko zosrothko merged commit 7d01116 into javacc:master Jun 6, 2021
@zosrothko
Copy link
Member

Thanks for you time spent in solving the Travis stuff. Woudl you agree to be the maintainer of this plugin since no one among the traditional contributors of JavaCC are knowledgeable enough with Gradle to maintain this plugin?

@kpitt kpitt deleted the gradle-6-upgrade branch June 6, 2021 12:33
@kpitt
Copy link
Contributor Author

kpitt commented Jun 6, 2021

Well, I guess I can give it a try. I'm not sure I'll have a wealth of time to apply to it, but it looks like it has been almost 3 years since the last active development so hopefully I can give it a little more than that.

@zosrothko
Copy link
Member

@kpitt Another important point missed. Could you update the README.md to reflect the change you made? TIA

@kpitt
Copy link
Contributor Author

kpitt commented Jun 12, 2021

I updated the README template file. The build is currently set up to regenerate the actual README file from the template as part of the release process. I can’t say I’m a fan of that approach, but I didn’t want to do anything that would upset the process too much as part of this initial PR so I just followed the lead of what John had been doing in his original 3.0.0 branch.

About the only thing that the template does is automatically insert the correct plugin version numbers into the instructions for installing the plugin, so it definitely feels like an unnecessary extra step. If you’d like, I can submit another PR that moves everything directly into the README and eliminates the extra template step.

@kpitt
Copy link
Contributor Author

kpitt commented Jun 12, 2021

I’m not sure what your plans are for releasing an update. Since 3.0.0 would be a major breaking release anyway, it might be a good time to consider a few other updates such as changing the plugin id to align with the JavaCC organization before putting out the final release.

@zosrothko
Copy link
Member

Aligning the JavaCC Gradle plugin with the JavaCC organisation is a very good point. Make the changes as you want, no problem. The organization prefix is: org.javacc. I would suggest to use: org.javacc.gradle as prefix, but feel free to choose the best one for the breaking release 3.0.0. Could you also manage to support JavaCC 8 version? Thanks

@kpitt
Copy link
Contributor Author

kpitt commented Jun 14, 2021

org.javacc.gradle works great for the Java package namespace. For the plugin id, I propose “org.javacc.javacc-gradle-plugin” on the Gradle Plugin Portal and “org.javacc:javacc-gradle-plugin” on Maven Central. I’ll try to get a pull request together for it next week.

The plugin has a default JavaCC version (currently at 6.1.2) but allows the user to define a different version to use. Version 7.0.10 should definitely work with no problem because it follows the same structure as 6.1.2, so 7.0.10 might be a good version to target as the new default for 3.0.0 so that we can get something out quicker.

I’ll test with JavaCC 8.0.1 to see if the plugin can handle the multiple artifacts needed for the generator plugin structure. If additional updates are needed for JavaCC 8 support, that would be a backward-compatible upgrade that could be released as 3.1.0, but if the changes are minor enough then I’ll try to get them in for 3.0.0.

I’ll create issues for both of these so that we can track the progress.

@zosrothko
Copy link
Member

If you don't mind, I would prefer org.javacc.gradle.javacc-gradle-plugin since there is already an org.javacc.maven prefix for the maven plugin.

Rest of you proposal is fine

@kpitt
Copy link
Contributor Author

kpitt commented Jun 14, 2021

I don’t see an org.javacc.maven group on Maven Central, but I do see org.javacc.plugin:javacc-maven-plugin. Is that what you meant?

https://github.com/javacc/javacc-maven-plugin/blob/master/pom.xml

@kpitt
Copy link
Contributor Author

kpitt commented Jun 14, 2021

Added issue #52 for changing the plugin ids, and issue #53 for JavaCC 8 support.

@zosrothko
Copy link
Member

org.javacc.plugin:javacc-maven-plugin
Yeap, that(s what I wanted to mean!

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

4 participants