Reorganize _ext projects

[k-brooks]

Reorganize the _ext projects as a Gradle multi-project build.
This will permit dependencies between the projects as well as potentially consolidating the version properties shared by projects.

[k-brooks]

Here's what I see as options
Option 1 - Move the _ext projects into their own repo, as they are treated as separate, independently versioned artifacts which are consumed indirectly by Spotless.
This will permit (simpler) automation when new publishing is needed, rather than requiring manual handling each time.

Option 2 - Composite build. The _ext folder is treated as it's own Gradle build and conditionally included as a composite in the root project. Task naming would be important (collisions) and dependencies within the root projects could be slightly more complex, but, it could streamline _ext development while maintaining the root project's independence for 'regular' development.

Option 3 - _ext folder remains, but treated as an independent Gradle build from root. Will permit interproject dependencies within _ext.

[nedtwigg]

There are two kinds of project in _ext. One is a glue code shim which is built and compiled normally (eclipse-base and eclipse-jdt). The other is a combination of glue code and a fat jar that bundles up a bunch of dependencies from Eclipse p2 repositories that are otherwise not available on mavenCentral (eclipse-cdt, eclipse-groovy, and eclipse-wtp).

You can tell which is which based on whether they have apply from: rootProject.file('../gradle/p2-fat-jar-setup.gradle'). If they do, then they are using the p2asmaven plugin to download a bunch of p2 dependencies and then smush them into a local maven repository. The big sin of this plugin is that this giant download is done at configuration time, not task time. I wrote the p2asmaven plugin, and this sin is committed for a reason, and it's very hard to fix.

I love how you have broken out the options. With the p2 fat jar problem in mind, here are the consequences:

• Option 1 will mean that any Spotless dev who wants to run any gradle task will first have to sit through a p2 download that will take minutes. These p2 repos don't have great uptime, and if they happen to be down, the build will fail and devs will have to know which part of the buildscript to magically comment out. I'm against Option 1.
• Option 2 and 3 both seem great to me. 2 seems strictly better than 3, so long as it turns out to be possible to get the composite build to work. If it's not, then 3 is still useful.
• Option 0, the status quo, does have one small advantage over 2 and 3. Which is that you don't have to run all of the p2 download tasks, only the one you care about. It's not a huge advantage though, and I think 2 or 3 is better overall.

An important challenge in this is that the interproject dependencies won't really work unless the fat jar part is taken into account. When you open the eclipse-cdt code, you're compiling against the jars in the local maven repo that you downloaded from p2. And when we publish, those local jars (and some even have nested jars!!!) all get mushed together into the final jar that gets uploaded to maven. And that mushed jar is required for the FormatterStep to work - it won't work without the fat jar step.

In general, we can release a new version of the JDT formatter just by bumping versions in the lockfile. We only need to change eclipse-base or eclipse-jdt if we want to change the shim code. At this point, they only ever change when @fvgh makes improvements to them, which have been increasingly minor improvements as time goes on (a testament to how successful his work on this has been! He's fixed a lot of things in a permanent way).

To release cdt, groovy, or wtp, it is rare that we need to change the shim code, we just need to publish a new fat jar containing the new deps, and then update the lock files.

So I think that the code in question is probably less-active than it seems at first. Regardless of how active this code is, the changes you suggest would definitely be a welcome improvement!

[k-brooks]

I think p2 is shorthand for painful squared....we have fought similar issues and ended up creating a static mirror for a number of p2 repos aggregated for each release of eclipse which we host internally. I wouldn't mind taking a look at making p2 resolution lazy...

I think you misunderstand what I meant with Option 1 - I am proposing that we remove the _ext folder from the Spotless repository and into a different (new), dedicated repository.
That would mean 'regular dev' of Spotless would not check this folder out as it is not included with the project, much less build/resolve the underlying artifacts.
We can move the source to a new repo and maintain the relevant history using a subtree split

[nedtwigg]

I wouldn't mind taking a look at making p2 resolution lazy

Here's the code. The problem (last I checked) is that gradle doesn't have APIs for pluggable dependency resolution. Since you can't download these deps lazily, the repository has to be ready for gradle to resolve them during configuration if it wants to. Deep in the issue history, there are links to a fork of gradle that someone built and briefly maintained to allow lazy p2 resolution.

Overall, eclipse is doing a good job moving more stuff to maven. All of JDT is now available on mavencentral, for example. I see p2 as a dead-end, and further investment in it is questionable, but up to you.

you misunderstand what I meant with Option 1

Yup, sorry, didn't read carefully enough. I know that @fvgh and I discussed splitting them into their own repo at one point and ultimately didn't do it, but I don't remember why. Whatever the reason was, it is a new problem now that we have to use a different old version of gradle to publish the _ext. And it would definitely be nice if it could be more automatic.

I would prefer that Spotless remained a monorepo. It is a good candidate for a split, but I don't see a compelling need. Perhaps we could achieve most of the CI benefits with a modestly more complex CI script, something like (my bash-foo is terrible).

if (git diff master _ext/** != 0) {
    ./_ext/gradlew check
} else {
    ./gradle check
}

[k-brooks]

With that in mind, I'll put a PR together for approach 2 - it will likely use a simple flag in rev 1, we could enhance it down the road with a source control check.

[k-brooks]

I took a stab at approach 2 + composite projects, but it created some issues WRT publishing.
Due to that, I am going to nix compositing the projects, and treat _ext as it's own project.

[nedtwigg]

How is it going? I think this approach is working fairly well for maven, I was thinking about adding something similar to slurp the _ext projects in.

[fvgh]

I provided a quick draft giving an idea what I have in mind.

Basic goals:

• Keeping _ext out per default (using approach suggested by @nedtwigg): As Ned pointed out, the _ext is not used by most developers and therefore the overhead should be omitted per default.
• Avoid duplicates in configuration: Initially the _ext projects had quite some differences in terms of building and testing. But time showed that most differences were unnecessary.

Why not composite builds or a standalone project?
Including (includeBuild) _ext as a composite builds has the disadvantage that _ext projects are considered independent of root. Hence it would for example not use the properties of root. As stated before, I think time showed that it makes no sense to decouple the build process entirely. We only had additional effort to maintain (for example upgrade to Gradle-6) and describe the builds.

Why not standalone repositories?
The _ext projects are standalone and used by projects like autostyle. But the development is currently not entirely independent. I consider the build and unit test only completely validated after the new _ext-plugin is passing Spotless root project tests.

[nedtwigg]

Your branch LGTM! I only saw one typo.

[fvgh]

@nedtwigg Sorry, think we have a misunderstanding. Branch was just meant to give you a quick idea how the thing should look like. The branch is missing quite a bit:

• Handling of lock files
• User documentation
• Eclipse integration
• CI testing

Anyhow, the branch key figures should remain. Loss of duplicated code.

[fvgh]

I will finalize the branch during this week. There is much more that can be improved with the latest Gradle version, like usage of Configuration. But if nobody disagrees, I'll provide a merge request which should close this issue.
Again, the build system is a living thing and there a lot of things to improve. My proposal just shall ensure that duplication vanishes and everything is compile-able with Gradle 6.

[fvgh]

Fixed by #519

[k-brooks]

Apologies for the laggy response :-)

In response to

How is it going? I think this approach is working fairly well for maven, I was thinking about adding something similar to slurp the _ext projects in.

Looks good to me, we do something similar in a few projects, but utilize properties to provide a simple/clean defaulting and override mechanism. We do it for both project inclusion, as well as plugin application (SonarQube and Jacoco come to mind).

The rest is just experience with 6.0 -
Our main driver to adopt 6.0 early was a StackOverflow bug in 5.6. Although we realized some nice improvements in plugin management, a lot of that stems from our need to use internal maven repositories (probably not applicable here). We've converted most of our internal plugins to deferred config, but we have not realized a huge gain, as many of our internal plugins are not configuration-heavy.
I did find that we played whack-a-mole with the deprecation warnings; fix one, and two new ones pop up. I think this project has already tackled the biggest hurdle we faced, which was the deprecated configurations in the java plugin (api vs compile, etc).
A 5.6 feature that we have been using, which I could see an application for here is https://docs.gradle.org/current/userguide/java_testing.html#sec:java_test_fixtures

[nedtwigg]

Ooh, thanks for pointing out the test_fixtures gizmo! That would let us subsume testlib into lib, and it's always been a bit confusing that lib doesn't have any tests (since they all live in testlib).