FREEMARKER-204 Change build from Ant to Gradle

[kelemen]

For easier review:

• Many files were moved around for easier compilation (i.e., there is no need to add file filters due to different dependencies)
  • Most files were just moved without any changes
  • Some tests were added a @RunWith annotation, because otherwise Gradle would skip those JUnit 3 tests.
  • A few java files had their mode changed from 755 to 644
  • The findbugs dependency of SuppressFBWarnings was removed from the normal source compile time classpath, and new package private annotations were created (since spotbugs only cares about the name, not the package). This change was made to avoid fooling up-to-date checks, and to simplify the build considerably.
  • FileTestCase.java was adjusted because of the resource path change. The current solution is still a hack, but a correct solution would require a change in many places (in tests only).
  • TemplateTest.java needed a small Javadoc change since it can no longer access TemplateTestSuite.
  • Other than the above, the osgi.bnd and rat-excludes files had to be adjusted due to file path changes (and new files).

The project can be build by executing:

./gradlew build -PsignPublication=false

The above command will build all packages and run all checks and tests.

To build with signatures, you can omit the "signPublication" property setting, and have to configure the gpg keys in ~/.gradle/gradle.properties like this:

signing.keyId=xxxxxx
signing.password=xxxxxx
signing.secretKeyRingFile=xxxxxx

A small improvement is that you can set the "developmentBuild" property to "true" in ~/.gradle/gradle.properties. In which case the timestamp will be set to the EPOCH (instead of the current time). This is beneficial to avoid preventing the jar task from being up-to-date. However, if this is made, then the official release has to pass -PdevelopmentBuild=false (a further improvement could be added to make a check depend on a task checking that this flag is not true).

To publish the artifacts to the Apache repository, the publish task has to be run as well (preferably in the same command as the "build" command).

[ddekany]

There were a few changes in the 2.3-gae (Ant) branch since then, because of which this PR will have to be updated (maybe after the other PR-s were merged).

• 2.3-gae is now built with Java 16 (because that's where Record support has become official)
• Java 7 support was removed, new minimum is 8
• rmic is not used anymore (because rmic was removed starting from JDK 15, and I don't think anyone need those generated debug API RMI stubs anymore)
• Javadoc customization was dropped, as it is not worth the complication anymore (like who read javadoc in browser nowadays, also the standard javadoc style is less horrible than it was in Java 8, even if still quite bad) - sorry about the wasted work there. (Already done in the 3 branch Gradle build)
• Javadoc now passes the default Java 16 HTML lint rules. In the 3 branch I have updated the javadoc linting parameters accordingly, same is needed here.
• Jetty used in tests was upgraded to one that's still supported, and is compatible with Java 16. That was also done in 3, also there the minimum Servlet/JSP was increased. But here it remains the same (so it's somewhat suboptimal that this Jetty version tests Serlvet 3.1 and JSP 2.3, but, no big deal)
• Github actions was added in the Ant build, which tests the "dist" task as well

[ddekany]

All other pending PR-s (that we afraid to break) has been merged. So, if the above were addressed, we can merge this.

[kelemen]

I have rebased the branch on top of "2.3-gae", but I haven't done meticulous validation yet after the rebase (only basic checks that it is not obviously broken). Anyway, I have pushed it so that you can review it (in the mean time, I will do more comparisons between the Ant and Gradle builds).

Also, I don't think I fully understood what you meant by "default Java 16 HTML lint rules" for Javadoc, because I have not seen anything special in the build.xml. So, please do check, if the current Javadoc settings are fine or not.

[ddekany]

Also, I don't think I fully understood what you meant by "default Java 16 HTML lint rules" for Javadoc, because I have not seen anything special in the build.xml. So, please do check, if the current Javadoc settings are fine or not.

As the Java version used for javadoc was increased to 16, the javadoc HTML lint rules become stricter, and therefore the javadoc comments were corrected (not build.xml). In the 3.0 branch Gradle buildSrc all the lints were accidentally disabled, so there I changed it to addStringOption("Xdoclint:all,-missing", "-quiet").

[kelemen]

Great, because as a coincidence I applied the same flags as well (though I can't see them anywhere in build.xml, maybe I'm just blind).

Also - just for reference - the new build applies a (lot) newer version of bnd, and thus generates a different manifest. If it turns out to be an issue, then I do have a local branch where I tested it with an older version (same as the Ant build), and that generates the same manifest (though it makes the build more awkward since I have to call the Ant plugin in that case, which is inconvenient to parameterize).

[chrisrueger]

added some thoughts on reproducible builds @kelemen

[chrisrueger]

@kelemen

I recently had to do with reproducible builds.
In this context it might be worth considering adding the following bnd properties:

-reproducible: true
-noextraheaders: true

See here and here

[kelemen]

Makes sense, but in this PR, my goal is to reproduce the Ant build the best I can. So, I think it would be better to have such changes be done in a separate PR (after merging this). And I guess, then it would be better, if someone more competent in OSGI (I myself have very limited experience and knowledge) did that (I assume you are a lot more competent than me in OSGI).

[chrisrueger]

@kelemen
also about reproducible builds (see my other comment)

Something like this might be worth considering for a reproducible build.

tasks.withType<AbstractArchiveTask>().configureEach {
  isPreserveFileTimestamps = false
  isReproducibleFileOrder = true
  filePermissions {
    unix("rw-r--r--")
  }
  dirPermissions {
    unix("rwxr-xr-x")
  }
}

Background info here

[kelemen]

Similar to your other comment, I think this would make sense in another PR as another request. It would also be less to verify in one go. Though, isReproducibleFileOrder = true might be something that would go well in this PR.

[chrisrueger]

@kelemen Sure, no problem, it can go in another PR. Just wanted to add my thoughts as it came to mind when reading this here and I thought it might be an idea "while you are on it" :) But sure there is already a lot in this PR, so feel free to do whenever you see fit.

[ddekany]

(though I can't see them anywhere in build.xml, maybe I'm just blind).

There's no -Xdoclint in the Ant build, and then by default javadoc does HTML lints.

[kelemen]

I did some deeper checks on the output of the Gradle build by comparing it to the output of the Ant build. The differences are the following:

• OSGi manifest entry differences due to the newer version of bnd being used as described previously.
• The Include-Resource manifest key have different value, but to be honest, I think this manifest entry does not make any sense in any case (I don't know why bnd adds it).
• There are a couple manifest attributes that I did not add: DSTAMP, TODAY, TSAMP. I'm not sure what step of the Ant builds adds these, but I can't really see the value in adding them. That said, they are trivial to add, if neccesary.
• As described in the opening comment of the PR, I have added a couple of package private SuppressFBWarnings classes to avoid the need to preprocess the source files. While this would be possible to do, but it could potentially fool IDEs and up-to-date checks.
• The generated class files are not exactly the same, because - if I understood the Ant build correctly - the Ant build uses the javac of JDK 16, just replaces the boot class path to compile most of the sources, while in this Gradle build I'm compiling those files with the javac of JDK 8. It would be technically possible to do the same in Gradle, but I'm not sure that it would be possible to tell the IDE then what is going on exactly.

[ddekany]

It's of course better that here we build with real JDK 8 (except core16, obviously).

The javadoc we are using is from JDK 8 currently, but it should be the JDK 16 one (that has the search field, etc.).

Signing: Not sure how people enter/store safely their private key password (like maybe have a gradle.properties on encrypted folder), but the easiest way I have found is just using gpg (it pops up a password prompt window), so I suggest adding this:

    if (hasProperty("useGpgCmd")) {
        signing {
            useGpgCmd()
        }
    }

(Haven't tested publishing to the Apache Maven repo yet. That also will get the extra credentials from somewhere. Will see...)

Some minimal comments/explanation here and there would be good. Like is's a quite tricky build with the "fake" subprojects for example. Also, in the 3 branch I had comments about the Serlvet/JSP testing dependencies, that weren't copied over.

The build section of the README.md needs to be updated. I can do it after merge, or maybe you want to provide some information yourself there.

[kelemen]

Right, I forgot to allow configuring credentials for the actual release repository ... I will fix that.

Javadoc JDK version: That is what I had in mind, just a couple of things became obsolete after the drop for Java 7 support, and I didn't reorganize those versions (and properties). I will clean this up, and will put a gradle.properties in the root with the default value rather than having them buried inside buildSrc.

I personally just keep a copy of my gpg file with a random password, and put that random password into the global gradle.properties. At least in this case this password can be easily changed, and is not something I would use for anything else. The main issue here is, if I were to lose the disk, but in that case I guess only disk encryption would save me. Anyway, the signing can already be disabled by the signPublication=false property, and if you prefer so, then I could change it to useGpgCmd as well (or maybe even create an "enum" like Gradle property to select the way to sign the publication).

I didn't see your comments (didn't even check) on branch 3, but I will do so, and will copy the relevant ones.

As for the README: I can update things from branch 3, and whatever you find missing you can update it after merge (it is probably easier than going back and forth in the PR for the README only).

[kelemen]

I have added the possibility to configure the credentials for publishing into the Apache repository by setting the freemarker.deploy.apache.user and freemarker.deploy.apache.passsword Gradle properties.

Also cleaned up the JDK version configurations (the default values are in gradle.properties).

Instead of having a signPublication boolean property, there is now a freemarker.signMethod property which can be set to none (default), gradle_properties (the previous signPublication=true behavior governed by the signing.xxx properties) or gpg_command (to apply the useGpgCmd configuration).

I have checked branch 3, but couldn't find your comments. Anyway, I have updated the README (the Eclipse part was a bit lazy, because I don't have Eclipse installed at the moment, I just simply wrote "Import the project as any other Gradle project", since - as I remember - there was nothing special when I tried last time.

[ddekany]

The artifact name on this branch should be org.freemarker:freemarker-gae, not org.freemarker:freemarker (which is just wrong, but... history.)

Other than that, it seems mergeable. (Also tried publishing to the snapshot repo of Apache.)

That comment on 3 branch is here (but obviously I can add it after merge too):

freemarker/freemarker-servlet/build.gradle.kts

Line 41 in 911f8fa

// Chose the Jetty version very carefully, as it should implement the same Servlet API, JSP API, and EL API

[kelemen]

Added the comments, and changed the artifact name to freemarker-gae. I was considering two options:

1. Change the name of the project.
2. Change only the name of the deployed artifacts.

I decided to go with option 1, because it is considerably more simpler, and less error prone. The reason I have considered option 2 is that in that case it wouldn't change the name of the project when switching branches. I'm not aware of any IDE issues when you changing the name of the root project though.

[ddekany]

Did some minor README(-ish) adjustments post-merge. Also this is not merged into the 2.3 branch.

One thing I noticed... you exclude gradlew from the source distro. I can't remember, were there some legal issues with including that? Because the goal would be that people can extract the src distro, look at the README, and can build the product.

[kelemen]

I'm not aware of any legal issues, but if you want to include gradlew scripts, then you also have to include the gradle-wrapper.jar which looks weird in the sources distribution. Besides, it is possible to build, if you have Gradle installed (since you can just use gradle instead of gradlew. Granted, the README assumes gradlew, but assuming someone wants to rebuild from this zip package (which is almost unthinkable for me) can easily do it. I mean, if someone can't figure out this much, then that person is so incompetent that what are we expecting of him/her to do in the first place?

[chrisrueger]

Quick feedback.
I built locally with gradle.

I added this to settings.gradle.kts

plugins {
    id("org.gradle.toolchains.foojay-resolver-convention") version "0.7.0"
}

as described here https://docs.gradle.org/current/userguide/toolchains.html#sub:download_repositories

which automatically downloads the required JDKs. I did not have JDK16 installed so the build failed initially. This plugin made it work automatically. Just as an idea to make it build right away without manual intervention.

Furthermore I installed built freemarker-gae-2.3.33-SNAPSHOT.jar into our OSGi environment and ran testcases successfully. So no regression on the part covered in our tests.

Conclusion: 👍

[kelemen]

@chrisrueger Thanks. I was bit worried that the new bnd might break something, but it is certainly reassuring that a real project is fine with the changes.

As for auto provisioning: I thought about applying it, but then decided against it since it can be annoying that a tool just installs a full JDK hidden deep into ~/.gradle. And one JDK you need anyway to start Gradle (although the current build works with any JDK 8 and above). Though since JDK 16 is uncommon for someone to have it already installed, it might be reasonable to have this automated, but I guess @ddekany can weigh in, if that would be a preferred behavior.

[ddekany]

I'm not aware of any legal issues, but if you want to include gradlew scripts, then you also have to include the gradle-wrapper.jar which looks weird in the sources distribution.

I added all gradlew related files, except the jar, but added a note about that in the README. It looks absurd, but it's ASF policy thing. When people vote for a release, they have to build from the source release, not from the Git repo sources. I checked what a few other projects did, and basically they do the same.

[ddekany]

@chrisrueger Thanks. I was bit worried that the new bnd might break something, but it is certainly reassuring that a real > Though since JDK 16 is uncommon for someone to have it already installed, it might be reasonable to have this automated, but I guess @ddekany can weigh in, if that would be a preferred behavior.

As an option we can do that. Easier for the people who will vote on the release.