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

Default to Java 11 for jdkVersions #897

Closed

Conversation

jonesbusy
Copy link
Contributor

@jonesbusy jonesbusy commented Jan 7, 2025

Fix #514 but maybe the opinion is different 2.5 years after

There are still few plugin still using implicit configuration which default to Java 8 still.

Those plugin are difficult for non-maintainer or non-administrator to modernize because it require the Jenkinsfile to be replayed

It would broke some plugin maybe (some are already broken) but I will argue it will benefit a lot modernizing many plugin to Java 11 and above (In more less 6 month Java 17 will be the recommended version for plugin bom in any case)

If for some some plugin still need to build on Java 8 they can move to explicit configuration until Java 8 is completely removed from ci.jenkins.io

Any though ?

@jonesbusy jonesbusy requested a review from a team as a code owner January 7, 2025 08:58
@jonesbusy jonesbusy force-pushed the feature/implicit-java-11 branch from 404e3d5 to 3e38acb Compare January 7, 2025 09:03
@jonesbusy jonesbusy force-pushed the feature/implicit-java-11 branch from 3e38acb to cd3617a Compare January 7, 2025 09:06
@dduportal
Copy link
Contributor

Thanks for the PR @jonesbusy ! Looks a good idea.

The SRE in me says it is a good idea, but this is a collective decision to be made: are you ok to add this to the agenda of an upcoming SIG platform please?

=> additionally, it require a bit more work, as we should identify the plugins which will be broken to make sure we don't have bad surprises. Maybe we can accept that this "work" would be "merge this PR, trigger build on all ci.jio plugins and check which one are failing". This won't be exhaustive (as not all plugins are using ci.jio) but it would cover the most used plugin as far as I know.

Any thoughts other key players here @basil @MarkEWaite @daniel-beck @Wadeck @jglick @jtnord @Poddingue @timja @mawinter69 @slides ?

@dduportal
Copy link
Contributor

(maybe also start discussion / or link to existing one on the developer mailing list to have a wider audience as this repo might not be followed by key maintainers)

@jonesbusy
Copy link
Contributor Author

Sure thanks.

Also from what I identified is that a lot of outdated plugin using implicit buildPlugin JDK 8 are already faling. (All that still depends on Jenkins 1.x and parent 1.x or 2.x. Because some old parent pom have unfixed range version for some tooling [1, ] like maven plugins. Which will pull a minimum required JDK version 17 for such plugins. (I don't remember if it's the hpi or some other tooling).

So a build that would have worked few years ago would fail now without any change

So for me the situation is that bumping to Java 11 as a default will not make the situation worse

I will try to find example or such unfixed version range on older plugin pom.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me, they'll likely still fail though due to the same problems with old parents etc though won't they? Although I guess this makes it easier to fix for non maintainers

@timja
Copy link
Member

timja commented Jan 7, 2025

are you ok to add this to the agenda of an upcoming SIG platform please?

I'm unsure if many of the involved people would attend that, I think either this PR or the mailing list would be most appropriate.

@jonesbusy
Copy link
Contributor Author

@gounthar
Copy link

gounthar commented Jan 7, 2025

I have a list of plugins without a Jenkinsfile, plugins using an old Java version, and plugins not defining a Java version, just in case. 🤷

@jtnord
Copy link
Contributor

jtnord commented Jan 7, 2025

Those plugin are difficult for non-maintainer or non-administrator to modernize because it require the Jenkinsfile to be replayed

if you are attempting to modernise a plugin though, why would you use a java 11 version over java17 which has been required since 2.462.1?

How is this different to a non maintainer modernising a plugin to support java21 / jakartaee or the like where the maintainer has explicitly put in versions?

Personally I am of the belief that the version should be explicit and the default should eventually die, and this PR makes this harder by keeping it alive. Whilst that makes it harder for non maintainers, unless we are constantly bumping the default (for java17 then java21) then we are just kicking a can down the road for the few that have not explicitly set the version.

I am not going to die on this hill.

@jonesbusy
Copy link
Contributor Author

jonesbusy commented Jan 7, 2025

though

Yes but at least PR that modernize them will not

if you are attempting to modernise a plugin though, why would you use a java 11 version over java17 which has been required since 2.462.1?

If I'm not wrong Java 17 is required since 2.479.1. We still recommend 2.452.4 which is still compatible with Java 11 (https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/#currently-recommended-versions) https://www.jenkins.io/doc/book/platform-information/support-policy-java/#running-jenkins-system

The reason we have an archetype Jenkinsfile with Java 17 and 21 is because we didn't detected issues that would occur on Java 11 and not 17 or 21 (This save also cost by removing a matrix from the build).

Don't get me wrong, I think those implicit version are bad and must be removed at some point.

The point is with a default on Java 8 it make it difficult to align them to the archetype and would cause many build issue for non-maintainer when opening PR (Which lower a lot chances for a PR gets merged)

My opinion is that this PR is a step further not a step back

@jtnord
Copy link
Contributor

jtnord commented Jan 7, 2025

If I'm not wrong Java 17 is required since 2.479.1. We still recommend 2.452.4 which is still compatible with Java 11 (https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/#currently-recommended-versions) https://www.jenkins.io/doc/book/platform-information/support-policy-java/#running-jenkins-system

Yup, my mistake.

@dduportal
Copy link
Contributor

Don't get me wrong, I think those implicit version are bad and must be removed at some point.

Mmmh, asking the question naively (as in: I don't see immediate obvious reason not to but I might be missing a LOT of things) but: since we are batch-modernizing plugins (with bunch of PRs cross repositories with corresponding tooling and processes), why not removing the default value and making builds to fail with an error message "please specify explicit version")?

If we have to break (or we already have them broken today) plugin builds, why not doing it once for all?

@gounthar
Copy link

gounthar commented Jan 7, 2025

Makes sense to me. 🤔
Better check the list of plugins building with the default version just in case...

@jonesbusy
Copy link
Contributor Author

Make sense, but perhaps it could be the next steps (maybe when 2.479.1 is the minimum recommended version ?

Maybe a less "big bang" approach

  1. We switch to default Java 11 on infra library
  2. We deprecate implicit settings and communicate (without faling build yet)
  3. We modernize as much plugin as possible using minimum Java 11 and Jenkinsfile archetype (Java 17 and Java 21) so that PR doesn't fail and anyone including non-maintainer can contribute
  4. We remove implicit settings, and fail build

What do you think?

Copy link
Contributor

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#897 (comment) I think we should just start failing the build now if you do not specify your environment explicitly. I do not see any benefit in trying to keep CI passing for new pull requests filed by contributors against completely unmaintained plugins; the first thing that needs to happen on such a plugin is that the maintainer merge a PR to update metadata files to something recent.

Alternately, just close this PR (which serves no purpose I can see), and do 2–4.

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.

Default buildPlugin() and buildPluginWithGradle() to Java 11
6 participants