java.level seems problematic, though. You may not define it to 11 when using a pre-switch jenkins.version.

Right, which is why this PR drops support for java.level=8.

To summarize the status at this time, there is:

• My proposal in this PR, which is feasible and implemented, but involves a drastic flag day (not ideal)
• @jglick's proposal to drop java.level from plugin pom.xml files in favor of inferring this based on jenkins.version, which seems feasible and does not involve a drastic flag day (seems better!), but which nobody has either implemented or volunteered to implement
• @oleg-nenashev's proposal to use a conditional profile activated with java.level, which as far as I can tell is not feasible so long as feedback like this remains unaddressed

Add […], and hard-code […]. And make sure that […].

Based on the use of the imperative mood in the above comments, I would like to clarify my intention when filing this PR. My intention was to do the minimum amount of work to reach a feasible result, which I believe I have accomplished. I am not against anyone doing a more complete and ideal implementation in another PR; however, my goal for this PR was merely to reach a minimum viable product (MVP).

Since it seems like the easiest option to implement, and cleans up tech debt, I am OK with a “flag day” switch like this, so that people updating to a post-switch jenkins.version could (must?) update the parent, while people sticking to a pre-switch jenkins.version must also stick with an old parent. (Or perhaps some backport versions, if we identify sufficiently important fixes we want everyone to adopt.) We do however need to ensure that if you inadvertently update the parent (e.g., Dependabot) while still using a pre-switch jenkins.version (and presumably <java.level>8</java.level>) that the build fails and that the error message is comprehensible, so devs will be forced to look at the release notes and decide what to do.

I do recommend that as part of this we drop the java.level property, which was never actually overridable in a meaningful way—you could make your plugin require Java 11 when core is 8-compatible but the Jenkins plugin manager running on 8 would AFAIK not protect users properly. This is clearly related, since otherwise we force people to change /properties/jenkins.version, /parent/version, and /properties/java.level in tandem. Anyway I suppose such a change should be very little work—just delete the property from pom.xml & ITs and hard-code 11 or 1.11 everywhere; optionally add a profile warning or failing if it is still defined. Anyone have more knowledge about this topic—whether there are plugins out there which must run on 11+ (I guess due to some library); and what the UX is like if you try to install that plugin when Jenkins is running on 8? Perhaps we can at least switch the policy to default java.level to the currently supported version, ensure the build fails clearly if it is set to something too old, but allow it to be overridden to something newer in the rare case that is required?

One other question. If we use the incompatible approach here, do we release it once the first weekly requiring Java 11 is released? Or wait for the first LTS after that? The former risks confusing devs with failing Dependabot PRs that would require a jenkins.version they are probably not interested in using yet; the latter might mean that plugins which do need a weekly dep would not have a working parent, unless they can use current parents with <java.level>11</java.level>. A middle way would be to release this as a -beta version which you can use if you know about it but which Dependabot will ignore.

I do recommend that as part of this

Is there any particular reason why cleaning up java.level needs to be done as part of this PR? Could not deleting java.level from pom.xml & ITs and hard-coding 8 or 1.8 everywhere be done in a separate PR and immediately merged/released without waiting for the Java 11 work?

Is there any particular reason why cleaning up java.level needs to be done as part of this PR?

Indeed no; my advice was more that it be done no later than this PR, so that devs picking up a new baseline & this PR are not forced to explicitly edit POMs to select Java 11 when that is really their only option anyway.

Could not deleting java.level […] be done in a separate PR and immediately merged/released without waiting for the Java 11 work?

Yes it could. Then this PR would just switch the hard-coded value from 8 to 11. Again pending some confirmation of real-world impact, that there is not a legitimate reason to override this to a different value than what would be implied by jenkins.version.

OK, so from a project management point of view we could define two work items: a cleanup task to remove the technical debt of java.level and an RFE to drop support for Java 8. The cleanup task does not depend on the RFE. The RFE is related to the cleanup task in that if the cleanup task is not completed first, then the technical debt associated with the cleanup task would be exposed in the adoption of the RFE. Therefore the RFE does not strictly depend on the cleanup task, but it would benefit from the cleanup task. The overall outcome would certainly be better if the cleanup task were completed first.

This PR represents the RFE. I think the cleanup task belongs in a separate PR which, as you note (and as is characteristic of technical debt), needs some careful research before it can be deemed safe, along with someone able and willing to deal with any potential regressions.

Whether the cleanup task should be considered a blocker for the RFE is perhaps a matter of subjective opinion. Certainly doing it first would result in a better overall outcome. But in my experience there is an emotional cost with doing such cleanup tasks and being on the receiving end of the associated regressions. Perhaps that is why some people call them "chores". I think these tasks are best distributed evenly among members of a team to avoid having a few individuals shoulder the bulk of the emotional cost. As you know, I do not shy away from such tasks, but I am cautious not to take on too many at once lest I become discouraged. Right now I am dealing with a systemd migration, so I am a bit hesitant to take on additional cleanup tasks. That may change in the coming days or weeks as I wake up with fresh motivation and a new set of priorities. I would not stop anyone else from working on this in the meantime, and I would love to mentor someone new who might be interested in helping out. To summarize, I will file the cleanup task in my TODO list as "would be nice to get to", but I would not consider it a blocker for the RFE.

Fine with me. Anyway we have some months (I guess?) before this is even eligible for merge.

We do however need to ensure […] that the build fails and that the error message is comprehensible, so devs will be forced to look at the release notes and decide what to do.

This can be done in org.jenkinsci.maven.plugins.hpi.ValidateMojo by resolving org.jenkins-ci.main:jenkins-bom:${JENKINS_VERSION}, checking the <properties> section (currently the only encoding of the minimum Java version for core is in the java.level property, inherited from the parent POM, but we can always remove java.level from the parent POM and add an explicit minimum Java version property to jenkins-bom), and then failing if the current JVM is too old.

Probably this property should never have been defined.

Agreed.

Perhaps we can deprecate it—ignore any value and issue a warning to that effect. Instead figure out what Java level to pass to tools based on the jenkins.version (somehow).

This might be possible with a new LifecyclePhase.INITIALIZE Mojo that reads the minimum Java version as described above and then dynamically adjusts the Maven configuration prior to proceeding onto the next lifecycle phase. Possibly… 😅

a new LifecyclePhase.INITIALIZE Mojo that reads the minimum Java version as described above and then dynamically adjusts the Maven configuration

This would likely work for Maven builds as such, but could well confuse tools such as IDEs which expect to evaluate a POM to understand basic aspects of project configuration. (In particular, what -source or -release argument would be used for Java compilation, thus the syntax accepted and the APIs available.)

Well I cannot come up with anything better so I am going to run with it.

Bear with me, I am trying to sort through the different options relating to java.level that have been discussed here, and refreshing my memory of the discussion. Am I getting it right?

1. Keep java.level and just say that new versions of the plugin POM will require it to be at least 11. You could handle that pretty simply with the Enforcer plugin or a profile: just fail validation if it is set to 8. (I think we can ignore other pre-11 values.) Something also needs to verify that the jenkins.version is sufficiently new. So someone seeing a new POM would also need to update two properties. Not great but OK. For better or worse, it preserves the ability for a plugin to request a Java version newer than what the corresponding Jenkins core requests.

2. Deprecate/ignore java.level, with whatever mojo issuing a warning, and hard-code the value as 11. Someone seeing a new POM would need to update jenkins.version (same as above), and delete java.level.

3. Report a build error rather than warning for java.level; otherwise as above.

4. Detect the Java level based on jenkins.version as you seem to be proposing. But since other parts of this PR really drop support for Java 8 (e.g. switching to -release) that would only be helpful at some point in the distant future when we introduce a version of Jenkins requiring Java 17, so that then we would not need to cut an incompatible parent POM release. In the meantime we risk breaking tools which inspect POMs. Does not seem like a great trade-off.

3 seems simplest and friendliest to me. If we want to remove java.level, and do so prior to requiring Java 11, then it might be preferable to do 2 now (compatible upgrade for anyone using <java.level>8</java.level>, just introduces a warning); and then harden it to 3 when doing the incompatible release which switches the default to 11 and requires a newer jenkins.version. I could try to draft such a change if there is consensus on what we want the result to be.

Yes, you read my mind regarding the last paragraph. That is exactly the conclusion I had come to.

Detect the Java level based on jenkins.version as you seem to be proposing.

I thought it was you proposing that? 😄 I have lost track at this point. But on a high level I think there is a valid point here: the minimum Java version is conceptually tied to the Jenkins core version, so encoding it in the plugin POM (which is in principle agnostic to core version, within a supported range at least) seems like the wrong place. And as a result of this impedance mismatch, you end up with the problem of incompatible flag days and more.

Let me give another example of the above: it's not just a matter of Java level. With Java 17 support, we're adding Add-Opens directives to MANIFEST.MF, and ideally I'd like Surefire to run the tests with the same Add-Opens directives as the plugin's core version ships with. Sure, we could hard-code some Surefire configuration in the plugin POM, but that has the same problem as hard-coding a Java level: it creates flag days, since if the directives change in core (as they did recently) the plugin POM would need to be updated in lockstep, yet the plugin POM is supposed to be agnostic to the core version.

So I think dynamically setting things like Java level and Surefire arguments based on the core version makes sense at a high level. It avoids the flag days, and it avoids duplication of information. Core can encode official values of things like the minimum Java level and the Add-Opens directives somewhere (e.g., in core-bom, or MANIFEST.MF in jenkins.war), and the build can then consume those based on the plugin's core version.

The challenge comes with tools, like you said. I wonder if we could blend both approaches: define a Java level and Surefire arguments in the plugin parent POM (corresponding to the minimum core version that the plugin parent POM supports) as a lowest common denominator and for the benefit of tools, but override those values dynamically in an initializer mojo based on the plugin's (not plugin POM's!) minimum core version. Maybe that way we could get the best of both worlds: precision for official builds, but degrading gracefully for GUI tools.

Speaking of which, do you think we have consensus on proceeding with #478 (comment) (i.e., a revert of JENKINS-20679)? I've prepared the commits for core, plugin-pom, maven-hpi-plugin, and update-center2, and they are extensive (hundreds of lines deleted), but I don't want to waste time testing and writing a description for Daniel about how backward compatibility will be maintained unless there is consensus that we want to go ahead with this.

I thought it was you proposing that?

Yeah I did speculate idly in #478 (review), though it does not seem like there is any way to do this in Maven Core (i.e. with a cleverly constructed POM). I am not even positive whether you can do this from a mojo early enough.

The “blend” approach would be nice if it worked. I am not too concerned about defining Surefire arguments dynamically since it is unlikely that tools would attempt to parse it out, unless there are some IDEs that bypass Maven to run tests yet read the POM (statically, without executing mojos) as the source of truth? But

IMO an incompatible release of the plugin POM is tolerable if it corresponds to a new Java major version requirement in core, which could arrive every couple years on average if we trail a fixed distance behind Java LTS releases. I agree that we would not want to be forcing a plugin POM range every time we adjust Add-Opens.

A more radical option would be to revert JENKINS-32493: resume packaging the plugin pom as part of core. This would avoid the mismatch problems not just of java.level but things like

do you think we have consensus on […] a revert of JENKINS-20679 (plugins requesting a newer version of Java than the corresponding core)?

I am not confident at this point. I have not personally searched for examples of plugins doing this, nor do I know of any examples offhand.

though it does not seem like there is any way to do this in Maven Core (i.e. with a cleverly constructed POM).

That I am fairly certain about, having tried many ways of doing this. I do not think it is possible.

I am not even positive whether you can do this from a mojo early enough.

Neither am I. Halfway through a prototype and it is challenging. 😅

The “blend” approach would be nice if it worked. […] I guess we would push out a POM update around the same time as the core release that switches the POM default, so IDEs would work again after a while. […] IMO an incompatible release of the plugin POM is tolerable if it corresponds to a new Java major version requirement in core, which could arrive every couple years on average if we trail a fixed distance behind Java LTS releases. I agree that we would not want to be forcing a plugin POM range every time we adjust Add-Opens.

Yeah agreed on all points.

A more radical option would be to revert JENKINS-32493: resume packaging the plugin pom as part of core.

Perhaps a bit too radical for me as part of this project. 😄

do you think we have consensus on […] a revert of JENKINS-20679 (plugins requesting a newer version of Java than the corresponding core)?

I am not confident at this point. I have not personally searched for examples of plugins doing this, nor do I know of any examples offhand.

OK so what are the next steps for getting people to agree about changes like this? Developer list thread? It is really frustrating to attempt a large body of work only to get nitpicked to death in code review from people who don't entirely agree with every minute detail and have no constructive suggestions.

Why was this closed? The last I understood from conversations in February was that we would release this in either June (right after the proposed weekly dropping Java 8 support) or September (the first LTS based on it). Did something change?

Intentionally […]?

This question implies that there is a problem with the PR (to be corrected by reinlining the profile). It does not state what the problem is or the reason why there is believed to be a problem. I will not participate in code reviews where reasoning is not provided.

This question implies that there is a problem with the PR

I was asking whether the change was intentional. Especially if you are not using

[merge]
	conflictstyle = diff3

(which GitHub Web does not!) it is easy to apply a hunk to the wrong side. But maybe the change was done for a good reason and just got included in a merge commit without a separate commit message. If I follow correctly, until 95cd21e (https://github.com/jenkinsci/plugin-pom/pull/478/files/22d4b09f2ea55dcf9b95e647a3ab2a401ccd0dee) we were switching (unconditionally, since no more java.level) from -source 8 -target 8 to -release 11 in the main POM config, and then also deleting the jdk-above-9 profile under the assumption that the JDK used during build must be at least 11. Now the behavior on JDK 11+ would be mostly the same (there is also some --add-opens?), but this is moved into a profile so if you try to build using JDK 8 you would…get a build failure somehow? Since maven-compiler-plugin will default to 1.5 source level IIRC?

I am still confused about why this PR is closed. Is there some replacement elsewhere?

Merging against #530 & #531 I guess, so just reintroduced the JDK 9 profile but this could be inlined for simplicity?

I was asking whether the change was intentional.

That question usually implies that the person asking the question believes there is a (potential) problem. If the person asking the question did not believe there was a (potential) problem, they would likely not be asking the question.

If you believe there is a problem with one of my PRs, please state what you believe the problem is and the reason why you believe it to be a problem. If the reason is valid, I will be happy to accommodate any changes. Do not ask me questions that imply there is a problem without stating it. Do not issue imperative statements requesting that I change X or Y without giving the reasoning for the request. I am being very clear about these boundaries.

I am still confused about why this PR is closed. Is there some replacement elsewhere?

I could ask the same thing about jenkinsci/jenkins#5304. As I wrote above, the reason this PR is closed is because you continue to comment on my PRs without providing reasoning. I am not interested in participating in such reviews.