Pull #14954: Add upwards compat for deprecated classes in maven-check… #14954

michael-o · 2024-06-07T15:51:38Z

The Maven Checkstyle Plugin cannot upgraded to 10.x at the moment, but users want to swap 9.x for 10.x at runtime. Add a new, deprecated constructor to SarifLogger to make that possible.

This covers apache/maven-checkstyle-plugin#141

…aven-checkstyle-plugin

michael-o · 2024-06-07T15:53:41Z

osipovmi@deblndw011x:~/var/Projekte/maven-checkstyle-plugin (master *=)
 git diff
diff --git a/pom.xml b/pom.xml
index adff04a..bd7db55 100644
--- a/pom.xml
+++ b/pom.xml
@@ -76,7 +76,7 @@ under the License.
     <javaVersion>8</javaVersion>
     <mavenVersion>3.6.3</mavenVersion>
     <resolverVersion>1.4.1</resolverVersion>
-    <checkstyleVersion>9.3</checkstyleVersion>
+    <checkstyleVersion>10.18.0-SNAPSHOT</checkstyleVersion>
     <slf4jVersion>1.7.36</slf4jVersion>
     <doxiaVersion>1.11.1</doxiaVersion>
     <doxiaSitetoolsVersion>1.11.1</doxiaSitetoolsVersion>
osipovmi@deblndw011x:~/var/Projekte/maven-checkstyle-plugin (master *=)
$ tree ./target/local-repo | grep checkstyle
│   │       └── checkstyle
│   │           │   ├── checkstyle-10.18.0-SNAPSHOT.jar
│   │           │   ├── checkstyle-10.18.0-SNAPSHOT.pom
│   │           │   ├── checkstyle-8.15.jar
│   │           │   ├── checkstyle-8.15.jar.lastUpdated
│   │           │   ├── checkstyle-8.15.jar.sha1
│   │           │   ├── checkstyle-8.15.pom
│   │           │   ├── checkstyle-8.15.pom.lastUpdated
│   │           │   └── checkstyle-8.15.pom.sha1
│   │   │   │   ├── maven-checkstyle-plugin
│   │   │   │   │   │   ├── maven-checkstyle-plugin-3.4.1-SNAPSHOT.jar
│   │   │   │   │   │   ├── maven-checkstyle-plugin-3.4.1-SNAPSHOT.pom
...
[INFO] --- maven-invoker-plugin:3.6.1:verify (integration-test) @ maven-checkstyle-plugin ---
[INFO] -------------------------------------------------
[INFO] Build Summary:
[INFO]   Passed: 49, Failed: 0, Errors: 0, Skipped: 0
[INFO] -------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:36 min
[INFO] Finished at: 2024-06-07T16:35:30+02:00
[INFO] ------------------------------------------------------------------------

rnveach

I am against supporting new code which has been deprecated for 15 months. While it does seem we forgot to support an old version of the constructor during our change, our goal has always been to move forward and not support old things. We dropped Java 6 and 8 support, and have turned people away as we couldn't support them any longer. We will probably soon drop 11 support.

We still have runtime compatibility with the old maven plugin version. The only reason why we still have this compatibility with the plugin is because we have ingrained ourselves too much with the plugin. We have asked years ago for the plugin to update so we could move on and remove old code, but it hasn't really happened. Our goal now is eventually to break away from the maven plugin and create even more breaking changes. This case is just a sign of things to come. I see maven plugin now needs to decide how it will support versions as we break things further.

Even if we made this change and released it, you would only get it in 10.17.1. What does this mean for versions users want to use between 10.9.3 and 10.17.0 where this constructor didn't exist? Will they be unsupportable? Why do you need compile compatibility with 10.9.3+ if you still have runtime compatibility with us through our snapshot? Can't you just use 10.9.2 as the latest before the breaking change?

michael-o · 2024-06-07T16:23:47Z

Reference: apache/maven-checkstyle-plugin#136 (comment)

@motlin

michael-o · 2024-06-07T16:41:48Z

I'll get back to this next week.

rnveach · 2024-06-07T16:52:15Z

Reference: apache/maven-checkstyle-plugin#136 (comment)
java.lang.NoSuchMethodError:

This is a runtime issue. Other thread said compile compatibility is what was being looked for.

This also shows that if we released this, it would only be supported in 10.17.1 and users could not use versions between 10.9.3 and 10.17.0.

Why doesn't maven plugin drop support for these old versions and use 10.9.3 as the base of what will be supported in the next plugin release?

michael-o · 2024-06-07T16:56:59Z

Reference: apache/maven-checkstyle-plugin#136 (comment)
java.lang.NoSuchMethodError:

This is a runtime issue. Other thread said compile compatibility is what was being looked for.

This also shows that if we released this, it would only be supported in 10.17.1 and users could not use versions between 10.9.3 and 10.17.0.

That would be acceptable.

Why doesn't maven plugin drop support for these old versions and use 10.9.3 as the base of what will be supported in the next plugin release?

Because we cannot raise the plugin baseline to Java 11. All core plugins will remain on Java 8.

rnveach · 2024-06-07T17:32:42Z

Because we cannot raise the plugin baseline to Java 11. All core plugins will remain on Java 8.

However, CS cannot be used on a Java 8 RE (Runtime Environment) since 10.0 .

Also technically, you are already asking us to support 10.17.1 with releasing this PR, so I don't see how you can say you can't upgrade to 10.9.3 but can support 10.17.1. Can't you keep things as "Java 8" and use CS 10.9.3 similar to how you are doing now?

I am sorry I am asking a lot of questions. I am sure admins will talk more on this over the weekend, I am just 1 of 3. I am still leaning against this as in the future you will have more pains and less options to overcome them when we do break from using the plugin.

romani · 2024-06-08T01:20:53Z

@michael-o ,
What if you do two releases sequentially:

based on checkstyle 10.0, just make it clear it is for jdk 11 binaries. No changes in plugin code about such Enum.
based on 10.9.3 when we introduced different class name. With changes in plugin to use new class name.

Second release will be in good position all further releases does not damage compatibility with maven plugin. So users will play with custom Checkstyle version freely.

romani · 2024-06-08T02:00:27Z

@motlin ,
For apache/maven-checkstyle-plugin#136 (comment)

It will be always a problem to ask be backwards compatible and need to use latest.
Checkstyle library doesn't support old versions, we going only forward.
If you need to stay on old versions of jdk, you can not use latest checkstyle library.
In few months we will bump min jdk to jdk17. So it will be checkstyle 11.0.0 and will be new big era.

motlin · 2024-06-08T14:28:43Z

@motlin , For apache/maven-checkstyle-plugin#136 (comment)

It will be always a problem to ask be backwards compatible and need to use latest. Checkstyle library doesn't need support old versions, we going only forward. If you need to stay on old versions of jdk, you can not use latest checkstyle library. In few months we will bump min jdk to jdk17. So it will be checkstyle 11.0.0 and will be new big era.

@romani just to clarify, I'm trying to use recent versions of everything: java 21, checkstyle 10.17.0, and maven-checkstyle-plugin 3.4.0. And I'm trying to set up SARIF output.

I'm one of the maintainers of https://github.com/eclipse/eclipse-collections/ where we go to great lengths to not break backwards compatibility so I can appreciate the effort here.

For folks who do want to use an old version of Java, they'd also have to use an old version of Checkstyle. It seems reasonable to me that they'd also have to use an old version of maven-checkstyle-plugin.

michael-o · 2024-06-08T16:09:54Z

Although I will provide an elaborated answer next week. All I wanted is to provide to as many users (like @motlin) as possible Maven Core plugins with a reasonable set of transitive deps, in other words: make as many people happy as possible since they give us a lot, like Eclipse IDE which I have been using for the past 20 years.

romani · 2024-06-08T22:58:09Z

I'm trying to use recent versions of everything

Yes, good decision. That why I suggesting to release Plugin 2 times to let people who can not use latest checkstyle library, just use not latest maven plugin.

michael-o · 2024-07-08T08:31:21Z

Guys, how to move forward with this? While I understand your sentiments, your limitations seems very arbitrary and biased to me. I try to include as many users as possible and -- as said -- moving to Java 11 is not an option at this time.
I see the following solutions:

Close this and lock people into 9.3
Merge and have upwards compat to 10.x at runtime to some extent
Move to https://github.com/rnveach/checkstyle-backport-jre8 (blocker is that it is not in Maven Central)
Remove SARIF support from the Maven Checkstyle Plugin

Note that I do not know what the technical motivation was to move Java 11 for Checkstyle.

michael-o · 2024-07-08T09:01:11Z

I am currently looking into option 3 with the given limitation put aside.

michael-o · 2024-07-08T09:42:43Z

Draft: apache/maven-checkstyle-plugin#143. Requires availability in Maven Central though.

rnveach · 2024-07-08T16:05:17Z

Note that I do not know what the technical motivation was to move Java 11 for Checkstyle.

You can read the discussion at #9146 . Basically more groups were moving to Java 11 and a discussion was had to move forward to 11 too.

There are already discussions to move to 17 soon.

your limitations seems very arbitrary and biased to me

From me, definitely not a bias to you. But I will admit we have have had hard times with the maven plugin. If you can't overcome this issue, things will get even harder when we break more compatibility in the future, which is planned.

Move to https://github.com/rnveach/checkstyle-backport-jre8 (blocker is that it is not in Maven Central)

There are no plans from me right now to put this into maven central. I am not against it, but it would be a big learn for me.

https://github.com/xvik/gradle-quality-plugin already uses backport. Maybe look at how they use backport to see if maven plugin can do the same. As far as I know, they link to the url in the plugin repositories ( see example in https://rnveach.github.io/checkstyle-backport-jre8/ ).

michael-o · 2024-07-08T16:26:20Z

Note that I do not know what the technical motivation was to move Java 11 for Checkstyle.

You can read the discussion at #9146 . Basically more groups were moving to Java 11 and a discussion was had to move forward to 11 too.

There are already discussions to move to 17 soon.

Scanned through the topic, there is zero mentioning about the technical benefits of 11, not to speak of 17.

your limitations seems very arbitrary and biased to me

From me, definitely not a bias to you. But I will admit we have have had hard times with the maven plugin. If you can't overcome this issue, things will get even harder when we break more compatibility in the future, which is planned.

Move to https://github.com/rnveach/checkstyle-backport-jre8 (blocker is that it is not in Maven Central)

There are no plans from me right now to put this into maven central. I am not against it, but it would be a big learn for me.

The release procedure is idential as to the actual checkstyle since the group id is the same. Take the config, apply, release, done.

https://github.com/xvik/gradle-quality-plugin already uses backport. Maybe look at how they use backport to see if maven plugin can do the same. As far as I know, they link to the url in the plugin repositories ( see example in https://rnveach.github.io/checkstyle-backport-jre8/ ).

Not an option because all artifacts on Central must be selfcontained. One cannot reference to a repo outside of Central. If it is not on Central then it does not exist.

rnveach · 2024-07-08T16:37:42Z

there is zero mentioning about the technical benefits of 11, not to speak of 17.

There was none. It was more about "keeping up with the kardashians" then looking for any new features.

The release procedure is idential as to the actual checkstyle

I don't do releases here. At one time we had to do manual releases, and I did it only once or twice. However, I never did the whole process from scratch, including applying for the name and new permissions, which is what backport will need since it is under a different organization.

motlin · 2024-07-08T16:46:10Z

There's a part of this conversation I don't understand. If I'm on a project where I want/need to use an old version of Java (like Java 8), I must use an old version of Checkstyle too (like Checkstyle 9). If I'm using an old version of Checkstyle, I already have plenty of versions of the maven-checkstyle-plugin which work for me - all the recently released versions including the latest.

Am I missing something? I was expecting that the way forward would be to release apache/maven-checkstyle-plugin#141 and to add documentation about which versions of the maven plugin are compatible with which versions of Checkstyle/Java.

michael-o · 2024-07-08T16:48:03Z

There's a part of this conversation I don't understand. If I'm on a project where I want/need to use an old version of Java (like Java 8), I must use an old version of Checkstyle too (like Checkstyle 9). If I'm using an old version of Checkstyle, I already have plenty of versions of the maven-checkstyle-plugin which work for me - all the recently released versions including the latest.

Am I missing something? I was expecting that the way forward would be to release apache/maven-checkstyle-plugin#141 and to add documentation about which versions of the maven plugin are compatible with which versions of Checkstyle/Java.

That PR was just a PoC.

michael-o · 2024-07-08T17:15:23Z

there is zero mentioning about the technical benefits of 11, not to speak of 17.

There was none. It was more about "keeping up with the kardashians" then looking for any new features.

That is quite disappointing and more or less needless. No itch, no scratch.

The release procedure is idential as to the actual checkstyle

I don't do releases here. At one time we had to do manual releases, and I did it only once or twice. However, I never did the whole process from scratch, including applying for the name and new permissions, which is what backport will need since it is under a different organization.

@romani Do you do the releases? Would you be willing to publish to Central?

rnveach · 2024-07-08T18:36:43Z

@romani basically does the releases for the CS organization. Backport is under my organization. CS didn't want to support anything as official since they only support the latest version. This is a main reason I don't support this PR, as you want to basically bring back support for what we are trying to remove.

Edit: Just adding Java 17 issue at #13209 . It has some other comments on why we want to move on to 17.

romani · 2024-07-09T00:22:38Z

We are small team, I try to not put more work on our team, we barely able to handle activities and support for main repo.
So backport exists for people who needs old stuff for longer, no support, nobody requested maven central until now, so means that no much needs, and people who needs old jdk are ok to build and put it in their private maven.

michael-o · 2024-07-09T05:59:19Z

We are small team, I try to not put more work on our team, we barely able to handle activities and support for main repo. So backport exists for people who needs old stuff for longer, no support, nobody requested maven central until now, so means that no much needs, and people who needs old jdk are ok to build and put it in their private maven.

I see, then the backport is not an option. 😟

…aven-checkstyle-plugin

romani

I am approving PR.
There is disagreement in team in this update.
But I reusing supper power to move forward with merge.

michael-o · 2024-10-26T07:13:21Z

I am approving PR. There is disagreement in team in this update. But I reusing supper power to move forward with merge.

Awesome, thank you.

romani · 2024-10-26T12:13:00Z

@michael-o , 10.19.0 is released please use it in plugin.

michael-o added a commit to michael-o/checkstyle that referenced this pull request Jun 7, 2024

Pull checkstyle#14954: Add upwards compat for deprecated classes in m…

8dd54e3

…aven-checkstyle-plugin

michael-o force-pushed the add-upwards-compat-maven-checkstyle-plugin branch from f0e5625 to 8dd54e3 Compare June 7, 2024 15:51

michael-o changed the title ~~Pull #14945: Add upwards compat for deprecated classes in maven-check…~~ Pull #14954: Add upwards compat for deprecated classes in maven-check… Jun 7, 2024

michael-o mentioned this pull request Jun 7, 2024

Checkstyle 10.x apache/maven-checkstyle-plugin#141

Closed

8 tasks

rnveach requested changes Jun 7, 2024

View reviewed changes

romani added the discussion ongoing label Jun 30, 2024

romani force-pushed the add-upwards-compat-maven-checkstyle-plugin branch from 8dd54e3 to 2e51881 Compare October 17, 2024 15:55

romani pushed a commit to michael-o/checkstyle that referenced this pull request Oct 17, 2024

Pull checkstyle#14954: Add upwards compat for deprecated classes in m…

2e51881

…aven-checkstyle-plugin

Pull checkstyle#14954: Add upwards compat for deprecated classes in m…

a43ccba

…aven-checkstyle-plugin

romani force-pushed the add-upwards-compat-maven-checkstyle-plugin branch from 2e51881 to a43ccba Compare October 18, 2024 13:01

romani approved these changes Oct 26, 2024

View reviewed changes

romani merged commit fdf8903 into checkstyle:master Oct 26, 2024
111 of 112 checks passed

romani added the miscellaneous label Oct 26, 2024

github-actions bot added this to the 10.19.0 milestone Oct 26, 2024

michael-o deleted the add-upwards-compat-maven-checkstyle-plugin branch October 26, 2024 07:12

romani removed the discussion ongoing label Oct 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull #14954: Add upwards compat for deprecated classes in maven-check… #14954

Pull #14954: Add upwards compat for deprecated classes in maven-check… #14954

michael-o commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach left a comment •

edited

Loading

michael-o commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach commented Jun 7, 2024 •

edited

Loading

romani commented Jun 8, 2024 •

edited

Loading

romani commented Jun 8, 2024 •

edited

Loading

motlin commented Jun 8, 2024

michael-o commented Jun 8, 2024

romani commented Jun 8, 2024 •

edited

Loading

michael-o commented Jul 8, 2024 •

edited

Loading

michael-o commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024

motlin commented Jul 8, 2024

michael-o commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024 •

edited

Loading

romani commented Jul 9, 2024

michael-o commented Jul 9, 2024

romani left a comment

michael-o commented Oct 26, 2024

romani commented Oct 26, 2024

Pull #14954: Add upwards compat for deprecated classes in maven-check… #14954

Pull #14954: Add upwards compat for deprecated classes in maven-check… #14954

Conversation

michael-o commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach left a comment • edited Loading

Choose a reason for hiding this comment

michael-o commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach commented Jun 7, 2024

michael-o commented Jun 7, 2024

rnveach commented Jun 7, 2024 • edited Loading

romani commented Jun 8, 2024 • edited Loading

romani commented Jun 8, 2024 • edited Loading

motlin commented Jun 8, 2024

michael-o commented Jun 8, 2024

romani commented Jun 8, 2024 • edited Loading

michael-o commented Jul 8, 2024 • edited Loading

michael-o commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024

motlin commented Jul 8, 2024

michael-o commented Jul 8, 2024

michael-o commented Jul 8, 2024

rnveach commented Jul 8, 2024 • edited Loading

romani commented Jul 9, 2024

michael-o commented Jul 9, 2024

romani left a comment

Choose a reason for hiding this comment

michael-o commented Oct 26, 2024

romani commented Oct 26, 2024

rnveach left a comment •

edited

Loading

rnveach commented Jun 7, 2024 •

edited

Loading

romani commented Jun 8, 2024 •

edited

Loading

romani commented Jun 8, 2024 •

edited

Loading

romani commented Jun 8, 2024 •

edited

Loading

michael-o commented Jul 8, 2024 •

edited

Loading

rnveach commented Jul 8, 2024 •

edited

Loading