search

spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

Support the Maven Toolchains Plugin #63

Open exabrial opened 5 years ago

exabrial commented 5 years ago

Spotbugs has a dependency on JDK 1.8. It's also not good to use JDK 1.8 as your primary JDK as it's showing it's age.

Fortunately, Maven has a plugin called the toolchains plugin which allows a different JDK to be used to run a plugin than the JDK that maven is running in. The user could be running maven in JDK 10, but then SpotBugs could be ran using a JDK 1.8 installation.

https://cwiki.apache.org//confluence/display/MAVENOLD/Toolchains

hazendaz commented 5 years ago

Spotbugs works on 9, 10, and 11. I only use 10 now for regular builds. Toolchains is overratted in my opinion and rarely if ever actually needed. I'm ok with the addition though and see the PR got the commits cleaned up after the first push. If this is ready and not a WIP as initially noted, can you rebase leaving the version alone and I'll include it in 3.1.4 release?

exabrial commented 5 years ago

@hazendaz Thanks a bunch! I cleaned up the PR and rebased, should be ready for your review

jansohn commented 5 years ago

@hazendaz I don't think toolchains is overrated but maybe I'm missing something. We have the following setup:

The project uses classes like javax.xml.bind.annotation.XmlRootElement which were removed from JDK11 onward. Now if we don't use the toolchain-plugin we get compilation errors like

error: package javax.xml.bind.annotation does not exist
error: cannot find symbol
  symbol: class XmlRootElement

If we use the toolchains-plugin to make sure the project gets compiled and built with JDK8 everything works fine until the spotbugs-plugin runs with the default JDK (because it does not support the toolchain):

[INFO] configuring report plugin com.github.spotbugs:spotbugs-maven-plugin:3.1.11
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.codehaus.groovy.vmplugin.v7.Java7$1 (file:/C:/Users/z200876/.m2/repository/org/codehaus/groovy/groovy/2.5.5/groovy-2.5.5-indy.jar) to constructor java.lang.invoke.MethodHandles$Lookup(java.lang.Class,int)
WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.vmplugin.v7.Java7$1
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
[INFO] Fork Value is true
     [java] The following classes needed for analysis were missing:
     [java]   javax.xml.bind.JAXBContext
     [java]   javax.xml.bind.Unmarshaller
     [java]   javax.xml.bind.Marshaller
     [java]   javax.xml.bind.JAXBException
     [java] Missing classes: 4
[INFO] Done SpotBugs Analysis....

As we can see it is also missing some JAXB classes which were removed from JDK11.

Personally, I think toolchain support would avoid these problems but maybe you can tell me a different way to fix the analysis?

hazendaz commented 5 years ago

Add a profile to run with jdk 9 and above with webservices-api latest from jakarta and all will be fine. As for groovy issue those warnings will persist but can be ignored.

Toolchains is rarely necessary. We target lower run times all the time with no issues. Use animal sniffer as well to keep newer code from leaking into tour source.

Get Outlook for Androidhttps://aka.ms/ghei36

From: jansohn notifications@github.com Sent: Wednesday, April 10, 2019 5:12:52 AM To: spotbugs/spotbugs-maven-plugin Cc: Jeremy Landis; Mention Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

@hazendazhttps://github.com/hazendaz I don't think toolchains is overrated but maybe I'm missing something. We have the following setup:

The project uses classes like javax.xml.bind.annotation.XmlRootElement which were removed from JDK11 onward. Now if we don't use the toolchain-plugin we get compilation errors like

error: package javax.xml.bind.annotation does not exist error: cannot find symbol symbol: class XmlRootElement

If we use the toolchains-plugin to make sure the project gets compiled and built with JDK8 everything works fine until the spotbugs-plugin runs with the default JDK (because it does not support the toolchain):

[INFO] configuring report plugin com.github.spotbugs:spotbugs-maven-plugin:3.1.11 WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by org.codehaus.groovy.vmplugin.v7.Java7$1 (file:/C:/Users/z200876/.m2/repository/org/codehaus/groovy/groovy/2.5.5/groovy-2.5.5-indy.jar) to constructor java.lang.invoke.MethodHandles$Lookup(java.lang.Class,int) WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.vmplugin.v7.Java7$1 WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations WARNING: All illegal access operations will be denied in a future release [INFO] Fork Value is true [java] The following classes needed for analysis were missing: [java] javax.xml.bind.JAXBContext [java] javax.xml.bind.Unmarshaller [java] javax.xml.bind.Marshaller [java] javax.xml.bind.JAXBException [java] Missing classes: 4 [INFO] Done SpotBugs Analysis....

As we can see it is also missing some JAXB classes which were removed from JDK11.

Personally, I think toolchain support would avoid these problems but maybe you can tell me a different way to fix the analysis?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-481609475, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AA7ho7y_nXlN4qYbFh5TQeYeifkep2whks5vfasUgaJpZM4Uk0dC.

jansohn commented 5 years ago

No, it's not as simple as that. The project isn't ready for JDK 11 (e.g. eclipselink https://github.com/eclipse-ee4j/eclipselink/issues/269) so our unit tests will fail if they are run with JDK 11. Also I want to run my unit tests with the same JDK version which I use in production.

The only ways to achieve to build and test our application are:

The toolchain option is way more flexible as it can be configured per project and does not affect the complete build environment. With .mavenrc option I can only build projects up to the specified JAVA_HOME JDK version.

I agree that toolchains is not necessary in every project but that doesn't mean it shouldn't be supported!

hazendaz commented 5 years ago

Thanks for the link. I was unaware anything in EE was still lacking jdk 11 support fully. I'd been using moxy with jdk 11 without issues but see things are more complicated still.

Is it possibly you could code a fix here for toolchain support?

Get Outlook for Androidhttps://aka.ms/ghei36

From: jansohn notifications@github.com Sent: Wednesday, April 10, 2019 7:34:59 AM To: spotbugs/spotbugs-maven-plugin Cc: Jeremy Landis; Mention Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

No, it's not as simple as that. The project isn't ready for JDK 11 (e.g. eclipselink eclipse-ee4j/eclipselink#269https://github.com/eclipse-ee4j/eclipselink/issues/269) so our unit tests will fail if they are run with JDK 11. Also I want to run my unit tests with the same JDK version which I use in production.

The only ways to achieve to build and test our application are:

The toolchain option is way more flexible as it can be configured per project and does not affect the complete build environment. With .mavenrc option I can only build projects up to the specified JAVA_HOME JDK version.

I agree that toolchains is not necessary in every project but that doesn't mean it shouldn't be supported!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-481653649, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AA7hozTU5-hMBQ5klN8Bfyrh9ChsidOPks5vfcxjgaJpZM4Uk0dC.

jansohn commented 5 years ago

There has already been an effort to achieve this: https://github.com/spotbugs/spotbugs-maven-plugin/pull/64

mathieucarbou commented 2 years ago

We also need this support.

At Terracotta we are using the toolchain plugin to ensure projects are built with a supported JDK, and tested against different JVMs. This is a good way to control the environment, ensure the right JDK / vendor are used, since a build platform or dev computer could have different JVMs installed and used by default. That's also not true that a project compiling with 8 for example will compile without any issue in 9 and 11 and so on.

There is a difference between the jvm used to run maven, the jdk used to compile and jvm used to run the tests. In the case of spotbugs, it would need to use the same jdk as the one used by the compiler plugin to give meaningful results and not crash with some "unsupported class" errors.

For reference, a colleague (@akomakom) did a PR in the surefire/failsafe plugin a while ago to add toolchain support to these plugins: https://github.com/apache/maven-surefire/pull/285/files

hazendaz commented 2 years ago

Can you provide an example of an actual issue? doesn't run 8nto unsupported class issues.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Mathieu Carbou @.> Sent: Saturday, April 2, 2022 6:32:18 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

We also need this support.

At Terracotta we are using the toolchain plugin to ensure projects are built with a supported JDK, and tested against different JVMs. This is a good way to control the environment, ensure the right JDK / vendor are used, since a build platform or dev computer could have different JVMs installed and used by default. That's also not true that a project compiling with 8 for example will compile without any issue in 9 and 11 and so on.

There is a difference between the jvm used to run maven, the jdk used to compile and jvm used to run the tests. In the case of spotbugs, it would need to use the same jdk as the one used by the compiler plugin to give meaningful results and not crash with some "unsupported class" errors.

For reference, a colleague @.***https://github.com/akomakom) did a PR in the surefire/failsafe plugin a while ago to add toolchain support to these plugins: https://github.com/apache/maven-surefire/pull/285/files

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-1086610964, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2IADNNUI65MZQJKJLVDAO3FANCNFSM4FETI5BA. You are receiving this because you were mentioned.Message ID: @.***>

hazendaz commented 2 years ago

As noted in here, if you need jdk 9+, add the api's to you project and continue using jdk 8. I don't think that is really an issue here. Having 100s of projects at my day job using jdk 17 to build and jdk 8 to run, toolchains isn't at all used, it should be fine. Unless there is something else I'm missing here. I don't believe so though.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Jeremy Landis @.> Sent: Saturday, April 2, 2022 12:07:14 PM To: spotbugs/spotbugs-maven-plugin @.>; spotbugs/spotbugs-maven-plugin @.> Cc: Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

Can you provide an example of an actual issue? This runs on numerous jdks already. It doesn't have issues itself as noted

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Jeremy Landis @.> Sent: Saturday, April 2, 2022 12:05:58 PM To: spotbugs/spotbugs-maven-plugin @.>; spotbugs/spotbugs-maven-plugin @.> Cc: Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

Can you provide an example of an actual issue? doesn't run 8nto unsupported class issues.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Mathieu Carbou @.> Sent: Saturday, April 2, 2022 6:32:18 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

We also need this support.

At Terracotta we are using the toolchain plugin to ensure projects are built with a supported JDK, and tested against different JVMs. This is a good way to control the environment, ensure the right JDK / vendor are used, since a build platform or dev computer could have different JVMs installed and used by default. That's also not true that a project compiling with 8 for example will compile without any issue in 9 and 11 and so on.

There is a difference between the jvm used to run maven, the jdk used to compile and jvm used to run the tests. In the case of spotbugs, it would need to use the same jdk as the one used by the compiler plugin to give meaningful results and not crash with some "unsupported class" errors.

For reference, a colleague @.***https://github.com/akomakom) did a PR in the surefire/failsafe plugin a while ago to add toolchain support to these plugins: https://github.com/apache/maven-surefire/pull/285/files

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-1086610964, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2IADNNUI65MZQJKJLVDAO3FANCNFSM4FETI5BA. You are receiving this because you were mentioned.Message ID: @.***>

mathieucarbou commented 2 years ago

Having 100s of projects at my day job using jdk 17 to build and jdk 8 to run, toolchains isn't at all used

That's the point.

The issue is happening when using toolchain because the spotbugs plugin does not obey the toolchain constrains.

Example:

> git clone git@github.com:Terracotta-OSS/angela.git
> cd angela
> ./mvnw clean install -DskipTests

You will see plenty of:

     [java] Exception in thread "main" java.lang.IllegalArgumentException: Unsupported class file major version 61
     [java]     at org.objectweb.asm.ClassReader.<init>(ClassReader.java:196)
     [java]     at org.objectweb.asm.ClassReader.<init>(ClassReader.java:177)
     [java]     at org.objectweb.asm.ClassReader.<init>(ClassReader.java:163)
     [java]     at edu.umd.cs.findbugs.asm.FBClassReader.<init>(FBClassReader.java:35)
     [java]     at edu.umd.cs.findbugs.classfile.engine.asm.ClassReaderAnalysisEngine.analyze(ClassReaderAnalysisEngine.java:48)
     [java]     at edu.umd.cs.findbugs.classfile.engine.asm.ClassReaderAnalysisEngine.analyze(ClassReaderAnalysisEngine.java:34)
     [java]     at edu.umd.cs.findbugs.classfile.impl.AnalysisCache.getClassAnalysis(AnalysisCache.java:261)
     [java]     at edu.umd.cs.findbugs.classfile.engine.ClassInfoAnalysisEngine.analyze(ClassInfoAnalysisEngine.java:75)
     [java]     at edu.umd.cs.findbugs.classfile.engine.ClassInfoAnalysisEngine.analyze(ClassInfoAnalysisEngine.java:38)
     [java]     at edu.umd.cs.findbugs.classfile.impl.AnalysisCache.getClassAnalysis(AnalysisCache.java:261)
     [java]     at edu.umd.cs.findbugs.ba.XFactory.getXClass(XFactory.java:685)
     [java]     at edu.umd.cs.findbugs.ba.AnalysisContext.setAppClassList(AnalysisContext.java:975)
     [java]     at edu.umd.cs.findbugs.FindBugs2.setAppClassList(FindBugs2.java:909)
     [java]     at edu.umd.cs.findbugs.FindBugs2.execute(FindBugs2.java:252)
     [java]     at edu.umd.cs.findbugs.FindBugs.runMain(FindBugs.java:395)
     [java]     at edu.umd.cs.findbugs.FindBugs2.main(FindBugs2.java:1231)

This happens when:

❯  ./mvnw -v
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /Users/matc/.m2/wrapper/dists/apache-maven-3.6.3-bin/lm9vem38rfmjij3jj0mk5bvnt/apache-maven-3.6.3
Java version: 17.0.2, vendor: Azul Systems, Inc., runtime: /Users/matc/.sdkman/candidates/java/17.0.2-zulu/zulu-17.jdk/Contents/Home
Default locale: en_FR, platform encoding: UTF-8
OS name: "mac os x", version: "12.3", arch: "x86_64", family: "mac"

Using the toolchain plugin to enforce compilation and test with JDK 1.8, but spotbugs seems to analyze some classes coming from the wrong jvm (17) instead of looking into toolchain for the right jvm that was used to compile the classes.

Attached a normal maven run with a run with -X and -e.

normal.txt debug.txt

There is a STRONG difference to make between the JVM used to run an application / tool like maven and the JDK used to compile the classes (and even eventually the jvm used to run the tests), which may not be the same.

Basically:

That's the correct way when one need a project to be compiled with an older jdk for compatibility reasons but tested on a different jvm to ensure the project will work and is supported on the tested versions, when a 1.8 compiled jar is provided.

It is not possible to use the same jdk to run maven, compile and test, except if you deploy jars per jdk version specifying that each jar is then only tested with the jvm used to compile. So foo.jar compiled in 17 would be tested with jvm 17 and then only has a guarantee to work with jvm 17 only.

Only is a string word but when working on low level projects there are a couple of changes from jvm to jvm that it might impact how the project works or behaves and its perfs.

hazendaz commented 2 years ago

@mathieucarbou Feel free to raise a PR. Some work was done in 2018 but never panned out.

However those are IMHO old considerations.

With jdk 9+, what animal sniffer used to do is built in. Its controlled by the release flags. Further using extra enforcer rules to prevent third party binary incompatibility which removed any need to use a separate jdk via toolchains. It won't cause any differences to your runtimes. You might get some tests differences with timestamps and other released items but nothing of any significance. In fact, the api added won't even be used with jdk 8 or before. Java requires endorced or extension to do so otherwise the classpath elements of the api are skipped before 9. Further setting them provided gives that extra comfort of it doesn't matter. As to surefire usage, you can split the entire build to use separate revision levels for tests to based on level you want to remove the minor differences if any. However, if your need is really insistent on toolchains, then feel free to add the support. I'm not against it just noting it is not required and has been effectively dispelled over many years. Some may still want to use it as you do and that is fine provided the support is provisioned to do so. If you want to see how this is achieved though on modern setup without toolchains, look at https://github.com/hazendaz/base-parent/ parent this project uses as it contains that support now and is in process of dropping jdk 8 entirely at this point (build with jdk 8 that is, will continue to target 8).

hazendaz commented 2 years ago

suggestion on that project you pointed out.

        <source>${java.build.version}</source>
        <target>${java.build.version}</target>

On the compiler plugin is ignoring all other plugins that use the same values. Also, if using jdk 17, its missing setup for release tags. Again looking at the project I noted, see how maven is done in proeprties section instead. I don't recall if that matters to spotbugs plugin but it might. There are so many that leverage the properties from maven compiler that it needs to be setup in the properties section to ensure other plugins get the same configuration. It may be that it doesn't realize jdk 8 was wanted. I didn't go up to the parent level on that project but looked at how compiler was being used to spot that consideration.

hazendaz commented 2 years ago

These are what control it more globally that other plugins further rely on.

        <maven.compiler.source>${java.version}</maven.compiler.source>
        <maven.compiler.target>${java.version}</maven.compiler.target>
        <maven.compiler.testSource>${java.test.version}</maven.compiler.testSource>
        <maven.compiler.testTarget>${java.test.version}</maven.compiler.testTarget>

note this is split source/test and doesn't need to be. The test ones will use the main ones.

If using jdk 11+ (really 9+), the following can come into play.

<maven.compiler.release>${java.release.version}</maven.compiler.release>
<maven.compiler.testRelease>${java.test.release.version}</maven.compiler.testRelease>

Same thing here, the test one is optional. You may want to play around with that and see if the issue behaves any different.

mathieucarbou commented 2 years ago

Also, if using jdk 17,

We don't want to use 17. There is a difference between:

A) the JVM used to run Maven (17) B) the JVM used to compile the project (1.8, from toolchain speced by version and vendor) C) the JVM used to test the project (1.8 or 11, from toolchain, speced by version and vendor)

B et C are controlled through properties (in our case java.build.version and java.yesy.version). We are building and testing our projects with several combinations of test JVM and OS. But we always compile them with 1.8. So the build machines all have a toolchain file pointing to different JVMs that will be used by maven to compile and test, regardless of the JVM used to run maven (which MUST NOT be the one used to compile or test).

In my example, 17 is just the JVM used to run maven, and eventually the default jvm used by the system to run other tools. It doesn't have to interfere with project building or testing.

But the spotbogs plugin seems to miss a "jvm" arg here that would allow the plugin specify the right jvm to use when ant is launching the JVM for spotbugs.

hazendaz commented 2 years ago

@mathieucarbou I don't see that option available in the groovy docs.

However, the issue is far easer than that based on testing of this. The version of spotbugs used in 'angela' doesn't support jdk 17. The spotbugs plugin is built groovy which is built on ASM and requires that it support jdk 17. The errors are with the plugin rather than anything with compilation / testing as noted. I didn't notice that at first glance here but your stack trace shows that. This support was not available back when 4.0.0 was released in 2020 that terracota-parent provides. I do not believe this is relavent at all to toolchains as a result. Groovy runs on ASM and as such needed 9.1 which came in groovy 3.0.8 with spotbugs maven plugin at 4.3.0. Currently it supports up to jdk 18.

That said, as you have documented here rather well, you want jdk 17 to run the maven build but not compilation / tests. Spotbugs isn't running compilation or tests. Spotbugs is static analysis so it only cares about the source. Its running on the jdk it was launched on which makes sense. I tested this now and with jdk 8 or 11, your build works fine (toolchained to 8). Only with the jump to 17 does it start to fail like this (would have been same on 15/16). So bump to 4.6.0.0 for the plugin and 4.6.0 for spotbugs and it again. It does get 'EI_EXPOSE_REP' issues in angela common module. See below. Can you try to apply same and see if those are something to be fixed in current code of angela? I'm not 100% sure when those popped up but seems any version past 4.3.0 and varies in count.

image

mathieucarbou commented 2 years ago

The version of spotbugs used in 'angela' doesn't support jdk 17. The spotbugs plugin is built groovy which is built on ASM and requires that it support jdk 17.

Yest, I know, and this is exactly why, in the plugin, when starting the spotbug jvm through the ant task programmatically, the ant java task has to use the right jvm that was used to compile the project.

https://ant.apache.org/manual/Tasks/java.html

The jvm attribute is used to specify which jvm has to be used to spawn the java class, in this case , spotbug, and it has to be taken from the toolchain if toolchain is used to control the jvm used to compile.

This is not a solution to upgrade the spotbug library... The issue I have shows specifically that spotbug is looking at version 17 classes a spart of it's analysis since ASM tries to load java 17 classes.

It should never do that, that's a mistake that could lead to wrong results and a non reproductible build that depends on the jvm version used to run maven

The project was compiled with jvm 1.8 : it has to be analysed with the same 1.8 classes that it was compiled with.

Why would you want spotbug to load as part of it's analysis the classes from the jvm that is used to run maven ? This JVM has nothing to do with the project!

Spotbugs is static analysis so it only cares about the source.

I agree

Its running on the jdk it was launched on which makes sense.

I disagree: if spotbug cares about the sources, to analyze them it has to use the jvm used to compile them to ensure a better compatibility and also isolate the build for it to be reproductible.

This is totally fine to run with a spotbug version that is not supporting 17. The jvm used on one computer is only used to run maven and does not have to interfere with the build. On their system, devs can have 8,11, 17, 18, etc as set by default.

What is important is to make the build isolated and reproductible by controlling the jvm used within maven plugins.

And in the case of spotbug, it has to analyze the compiled classes with the same jvm used to compile, otherwise there is a dependency of the build on the jvm version used to run maven, which Is totally unacceptable for a reproductible build.

The jvm version used on dev computer to run maven must not interfere with the build.

hazendaz commented 2 years ago

I get ant allows that but groovy doesn't show it. Regardless I'm open to a PR but either way you need to upgrade to the latest which is only supported version. That in turn fails your code due to other spotbug compliance issues. If you can fix the new violations on latest that may just outright resolve your issue. Implying toolchain support everywhere sort of indicates you have an expectation all plugins do this and that simply isn't true. I've seen nothing here to sway my mind on this important key issue. Now I didn't write this original plugin. I wouldn't have used groovy at all but it is what it is. When groovy is used it is very sensative to the jdk used. If this where java you would not have even raised this issue. So please keep an open mind here. Try the supported version 4.6.0.0, raise a PR if needed but understand this is a static analysis not a compiler and understand the issue you face is directly due to insistence on using jdk 15+ to build where groovy doesn't support it.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Mathieu Carbou @.> Sent: Sunday, April 3, 2022 3:44:09 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

The version of spotbugs used in 'angela' doesn't support jdk 17. The spotbugs plugin is built groovy which is built on ASM and requires that it support jdk 17.

Uesy, I know, and this is exactly why, in the plugin, when starting the spotbug jvm through the ant task programmatically, the ant java task has to use the right jvm that was used to compile the project.

https://ant.apache.org/manual/Tasks/java.html

The jvm attribute is used to specify which jvm has to be used to spawn the java class, in this case , spotbug, and it has to be taken from the toolchain if toolchain is used to control the jvm used to compile.

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-1086796322, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODIYO5DIEYCWCLFGAYT3VDFD4TANCNFSM4FETI5BA. You are receiving this because you were mentioned.Message ID: @.***>

hazendaz commented 1 year ago

Can you provide an example of an actual issue? This runs on numerous jdks already. It doesn't have issues itself as noted

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Jeremy Landis @.> Sent: Saturday, April 2, 2022 12:05:58 PM To: spotbugs/spotbugs-maven-plugin @.>; spotbugs/spotbugs-maven-plugin @.> Cc: Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

Can you provide an example of an actual issue? doesn't run 8nto unsupported class issues.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Mathieu Carbou @.> Sent: Saturday, April 2, 2022 6:32:18 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

We also need this support.

At Terracotta we are using the toolchain plugin to ensure projects are built with a supported JDK, and tested against different JVMs. This is a good way to control the environment, ensure the right JDK / vendor are used, since a build platform or dev computer could have different JVMs installed and used by default. That's also not true that a project compiling with 8 for example will compile without any issue in 9 and 11 and so on.

There is a difference between the jvm used to run maven, the jdk used to compile and jvm used to run the tests. In the case of spotbugs, it would need to use the same jdk as the one used by the compiler plugin to give meaningful results and not crash with some "unsupported class" errors.

For reference, a colleague @.***https://github.com/akomakom) did a PR in the surefire/failsafe plugin a while ago to add toolchain support to these plugins: https://github.com/apache/maven-surefire/pull/285/files

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/63#issuecomment-1086610964, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2IADNNUI65MZQJKJLVDAO3FANCNFSM4FETI5BA. You are receiving this because you were mentioned.Message ID: @.***>

mathieucarbou commented 1 year ago

Can you provide an example of an actual issue? This runs on numerous jdks already. It doesn't have issues itself as noted Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg … ____ From: Jeremy Landis @.> Sent: Saturday, April 2, 2022 12:05:58 PM To: spotbugs/spotbugs-maven-plugin @.>; spotbugs/spotbugs-maven-plugin @.> Cc: Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63) Can you provide an example of an actual issue? doesn't run 8nto unsupported class issues. Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg ____ From: Mathieu Carbou @.> Sent: Saturday, April 2, 2022 6:32:18 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63) We also need this support. At Terracotta we are using the toolchain plugin to ensure projects are built with a supported JDK, and tested against different JVMs. This is a good way to control the environment, ensure the right JDK / vendor are used, since a build platform or dev computer could have different JVMs installed and used by default. That's also not true that a project compiling with 8 for example will compile without any issue in 9 and 11 and so on. There is a difference between the jvm used to run maven, the jdk used to compile and jvm used to run the tests. In the case of spotbugs, it would need to use the same jdk as the one used by the compiler plugin to give meaningful results and not crash with some "unsupported class" errors. For reference, a colleague @.https://github.com/akomakom) did a PR in the surefire/failsafe plugin a while ago to add toolchain support to these plugins: https://github.com/apache/maven-surefire/pull/285/files — Reply to this email directly, view it on GitHub<#63 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2IADNNUI65MZQJKJLVDAO3FANCNFSM4FETI5BA. You are receiving this because you were mentioned.Message ID: @.>

Please read the whole thread ;-) It's there.

hazendaz commented 1 year ago

Sorry @mathieucarbou not sure what is going on with my email. I sent that message not 4 days ago but back on 4/2 per my email history. Having messages show up on various tickets all 4 days ago so feels like some lost set of emails now showing up and seems to span all year. All seem to be from verizon so likely some bug on their end got fixed.

Anyway back on this highly unlikely this will be provided. I had saw on maven mailing list recently Maven team members stating how tool chains isn't necessary any more which was my basic points anyways. I'm not against support if someone with more knowledge on how that plugin works could provide it and understanding probably the major constraint here is using groovy ant which does not appear to provide api you noted ant does provide.

mathieucarbou commented 1 year ago

Sorry @mathieucarbou not sure what is going on with my email. I sent that message not 4 days ago but back on 4/2 per my email history. Having messages show up on various tickets all 4 days ago so feels like some lost set of emails now showing up and seems to span all year. All seem to be from verizon so likely some bug on their end got fixed.

Anyway back on this highly unlikely this will be provided. I had saw on maven mailing list recently Maven team members stating how tool chains isn't necessary any more which was my basic points anyways. I'm not against support if someone with more knowledge on how that plugin works could provide it and understanding probably the major constraint here is using groovy ant which does not appear to provide api you noted ant does provide.

Where I work, we are still supporting the branches of our products (and previous ones) that are more than 10 years old. Most of our projects need to be compiled with 1.8 (and not with a more recent one using compatibility flags), and can run on >= 1.8. We also need to run our project tests on 1.8, 11 and 17 to ensure everything works fine because we are doing low level stuff and dealing with off heap memory / misc.Unsafe.

That's the reason we are using toolchain, to be able to select the jdk to use per plugin. There is no other way to ensure a project can be compiled with version X and tests run with version Y. There are other plugins too that are obeying toolchain. As a rule, any plugin that are dealing with the compiled classes need to support changing it's jvm (like surefire and compiler are doing).

Maven itself is running with the java executable found in the path or java_home. There is strictly no requirement and no link at all between the jvm running maven and the jdk that should be selected for testing and compiling.

Example: we have some requirements to compile with Zulu and run our test with a selection of JVM and versions we can support on different OSs at customer sites.

But to run maven, the same jvm can be used.

Anyway.... If Maven and its plugins are closing more and more the door to support such use cases, I am then glad we switched some projects to Gradle, which by the way allows that flexibility.

hazendaz commented 1 year ago

Maybe the original PR from 2018 could be explored further. Seems it may have been close but needs github actions setup now. I'm willing to take a look at that effort.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Mathieu Carbou @.> Sent: Saturday, October 15, 2022 2:45:38 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Mention @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] Support the Maven Toolchains Plugin (#63)

Sorry @mathieucarbouhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmathieucarbou&data=05%7C01%7C%7C9ea959028ec84deb69b008daae78df88%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638014131417471983%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=uY4EvSQBFeRbVrcg1ABD8cTwKgFY8jkAXF8wNYnz6TA%3D&reserved=0 not sure what is going on with my email. I sent that message not 4 days ago but back on 4/2 per my email history. Having messages show up on various tickets all 4 days ago so feels like some lost set of emails now showing up and seems to span all year. All seem to be from verizon so likely some bug on their end got fixed.

Anyway back on this highly unlikely this will be provided. I had saw on maven mailing list recently Maven team members stating how tool chains isn't necessary any more which was my basic points anyways. I'm not against support if someone with more knowledge on how that plugin works could provide it and understanding probably the major constraint here is using groovy ant which does not appear to provide api you noted ant does provide.

Where I work, we are still supporting the branches of our products (and previous ones) that are more than 10 years old. Most of our projects need to be compiled with 1.8 (and not with a more recent one using compatibility flags), and can run on >= 1.8. We also need to run our project tests on 1.8, 1.11 and 1.17 to ensure everything works fine because we are doing low level stuff and dealing with off heap memory / misc.Unsafe.

That's the reason we are using toolchain, to be able to select the jdk to use per plugin. There is no other way to ensure a project can be compiled with version X and tests run with version Y. There are other plugins too that are obeying toolchain. As a rule, any plugin that are dealing with the compiled classes need to support changing it's jvm.

Maven itself is running with the java executable found in the path or java_home. There is strictly no requirement and no link at all between the jvm running maven and the jdk that should be selected for testing.

Example: we have some requirements to compile with Zulu and run our test with a selection of JVM and versions we can support on different OSs at customer sites.

But to run maven, the same jvm can be used.

Anyway.... If Maven is closing more and more the door to support such use cases, I am then glad we switched some projects to Gradle, which by the way allows that flexibility.

— Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fspotbugs%2Fspotbugs-maven-plugin%2Fissues%2F63%23issuecomment-1279677002&data=05%7C01%7C%7C9ea959028ec84deb69b008daae78df88%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638014131417471983%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ZSoTin2tH05Av82o8yY%2F6tf3KdXU5NnEbPBCfyeWPpo%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAHODI3WVYQR2I2N35ZOIALWDJHJFANCNFSM4FETI5BA&data=05%7C01%7C%7C9ea959028ec84deb69b008daae78df88%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638014131417471983%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WALvVtlK8AWZ7hOUbUE1fN%2FeEpcxhy5Wn7%2BCY87KLi0%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>