Added eclipse-wtp 3.15.2 to Eclipse (as pseudo Eclipse version 4.13.1).

[fvgh]

Fix #492.
Introduced pseudo Eclipse version 4.13.1 since the new approach does not support multiple WTP configurations (for the same WTP type). So the user still has the possibility to use 4.13.0.

If any user needs multiple WTP configurations, please raise an issue.

[nedtwigg]

Hmm, the changelog and documentation for this is a bit tricky. In general, I'm in favor of complete documentation, but in some cases I think it's better to have a descriptive error message, if it makes the documentation shorter and easier to read for the 95% usecase. As Spotless has grown more capable, I think our docs are starting to become a little less beginner-friendly.

The rationale behind the approach we took was that every known user is fine with just one config, so I think it's okay to leave it out of the docs, so long as the changelog and error has the required info to help debug any surprises. Here's another way to do the changelog:

• Formerly, the Eclipse-WTP formatter (web tools platform, not java) could encounter errors in parallel multiproject builds. This is now fixed, but there is a new limitation that WTP must have the same configuration for every subproject it is used in. For example you cant use eclipseWtp('xml').configFile('a') in one project and eclipseWtp('xml').configFile('b') in another. You also can't use eclipseWtp('xml') in one project and eclispeWtp('css') in another. If this limitation is a problem for your use case, please leave a comment in Remove single-config limitation from Eclipse WTP formatter #538

We do generate an error if it is used incorrectly, correct? Ideally we would put a link to #538 into that error, but I think it's also sufficient to just put the stacktrace into the body of #538 so that it is easy to google.

[fvgh]

Hmm, the changelog and documentation for this is a bit tricky. In general, I'm in favor of complete documentation, but in some cases I think it's better to have a descriptive error message, if it makes the documentation shorter and easier to read for the 95% usecase. As Spotless has grown more capable, I think our docs are starting to become a little less beginner-friendly.

Proposals are always welcome.

The rationale behind the approach we took was that every known user is fine with just one config, so I think it's okay to leave it out of the docs, so long as the changelog and error has the required info to help debug any surprises. Here's another way to do the changelog:

* Formerly, the Eclipse-WTP formatter (web tools platform, not java) could encounter errors in parallel multiproject builds.  This is now fixed, but there is a new limitation that WTP must have the same configuration for every subproject it is used in.  For example you cant use `eclipseWtp('xml').configFile('a')` in one project and `eclipseWtp('xml').configFile('b')` in another.

You also can't use eclipseWtp('xml') in one project and eclispeWtp('css') in another. If this limitation is a problem for your use case, please leave a comment in #538

The second part is incorrect. We already use different class loader for "different formatter types".
We could use the same option, as already foreseen, also to support multiple configurations.
In this scenario, I would waste some memory to statically cache properties hash sum and select the appropriate JAR state. This might of course stock pile old JAR state if the developer changes configuration. So there must be some limit and appropriate error if limit is reached.
So when the documentation get's too messy, maybe we just should adapt the implementation.

We do generate an error if it is used incorrectly, correct? Ideally we would put a link to #538 into that error, but I think it's also sufficient to just put the stacktrace into the body of #538 so that it is easy to google.

This is an option. This is the current error. The error was always there for CSS, since unlike the other WTP formatters, it does not (or at least I have not found it) provide a way to prevent reloading the configuration from the global WTP plugin preferences.

[nedtwigg]

Thanks, I forgot we had built some of that, in that case, maybe this as the changelog entry:

• Formerly, the Eclipse-WTP formatter (web tools platform, not java) could encounter errors in parallel multiproject builds. This is now fixed, but there is a new limitation which probably won't affect any users, but just in case:
  • eclipseWtp('xml') in one subproject and eclipseWtp('css') in another will work fine, but...
  • eclipseWtp('xml').configFile('a') in one subproject and eclipseWtp('xml').configFile('b') in another will not, and your build will fail with an exception. If this limitation is a problem for your use case, please leave a comment in Remove single-config limitation from Eclipse WTP formatter #538.

Except, unless I misunderstand, I actually don't think we have that limitation! As you pointed out, the this in JarState.getClassLoader(this) is referring to:

spotless/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java

Lines 143 to 151 in 11714b6

	public static class State implements Serializable {
	// Not used, only the serialization output is required to determine whether the object has changed
	private static final long serialVersionUID = 1L;
	private final JarState jarState;
	//The formatterStepExt assures that different class loaders are used for different step types
	@SuppressWarnings("unused")
	private final String formatterStepExt;
	private final FileSignature settingsFiles;

so now I think we are already capturing the settings, and there is no limitation. Am I missing something again?

[fvgh]

Mh, and I forgot that we really do a new state per config change. I only remembered the formatterStepExt I once introduced....
I don't like it very much, since it adds an unrestricted amount of new objects to a static cache.
Further more it should not be required for most Eclipse formatters.
I put it in as a copy and paste form the initial JDT formatter .
I don't say that it is responsible for problems such as #525, but it definitely seems a waste of memory to me. I have no idea why it was used for the original JDT formatter.

But your are right, we currently have no restrictions on WTP.
I even once programmed a test checking multiple configurations for WTP.

To summarize:

• I will provide a new PR for XML formatter config is sometimes not applied when running gradle --parallel #492.
• I will open an issue to clarify and cleanup the EclipseBasedStepBuilder

[nedtwigg]

I don't like it very much, since it adds an unrestricted amount of new objects to a static cache.
Further more it should not be required for most Eclipse formatters.

For every build tool I'm aware of besides Gradle, the build is a run-once process. Anything that might be useful is cached, since the whole process is just going to exit soon anyway.

The gradle daemon is super useful, but because it's long-lived there's a chance that the "cache-everything" approach can cause it memory-leak into oblivion. But I think that's not only okay, but also the right approach for build plugins to take. The build does not change often, and when it does change restarting the daemon is not a relatively-expensive process anyway. For the common case where the build does not change then our current approach is the most-efficient, easiest-to-use way to do it.

[nedtwigg]

I think this PR is fine as-is, it just doesn't need the doc changes, and can get by with a simpler changelog entry.