jonathanlermitage / oga-maven-plugin

:jigsaw: Old GroupIds Alerter - A Maven plugin that checks for deprecated groupId+artifactId (e.g. did you know that graphql-spring-boot-starter moved from com.graphql-java to com.graphql-java-kickstart?).
https://central.sonatype.com/search?q=biz.lermitage.oga
MIT License
39 stars 7 forks source link

check version #19

Closed delanym closed 3 years ago

delanym commented 3 years ago

Reduce false positives by checking on a per version basis. For example, I'm still using commons-collections:commons-collections:3.2.2. This should not be changed to org.apache.commons:commons-collections4:3.2.2

jonathanlermitage commented 3 years ago

Hi @delanym
I think this criteria is too broad. Also, commons-collections4 is the new location of commons-collections according to mvnrepository.com.

I would suggest something else: if you think this is a false positive, for any reason, we could implement a white-list (or an ignore-list, I don't know how we should name it exactly). We could configure a list of maven coordinates to ignore. Do you think it could feet your needs? Something like this:

<plugin>
  <groupId>biz.lermitage.oga</groupId>
  <artifactId>oga-maven-plugin</artifactId>
  <configuration>
    <ignoreList>commons-collections:commons-collections</ignoreList> <!-- list of coordinates (groupId:artifactId or groupId only), separated with commas -->
  </configuration>
</plugin>
delanym commented 3 years ago

Hi @jonathanlermitage I don't want to ignore all commons-collections:commons-collections because if there is a commons-collections:commons-collections:4, then that SHOULD be org.apache.commons:commons-collections4:4

On terminology, its not that any version of this artifact has actually changed coordinates. Subsequent versions of this artifact are published under a different groupid/artifactid. If the version of the artifact I was using was republished under different coordinates, then I would want to update the coordinates. Otherwise there's nothing to do.

So 2 possibilities:

  1. If I'm using an artifact which is available under 2 sets of coordinates, and I'm using the deprecated one, I expect an error.
  2. If I'm using an older version of an artifact, and the newer version's coordinates have changed, I would like a warning that future versions of the artifact have "moved", and I need to update the dependency manually.
jonathanlermitage commented 3 years ago

Thank you very much for these explanations :+1:
Yeah, it would be a nice feature. We could probably reuse some code from versions-maven-plugin to identify newer versions of an artifact.

froque commented 3 years ago

I don't understand how this could be a false positive.

The goal for this plugin is to detect artifacts that were relocated and commons-collections:commons-collections was relocated to org.apache.commons:commons-collections4.

Almost all relocated packages that I analyzed were relocated along with a new version.

The only package I recall to be publishing to both old and new coordinates was hibernate-core in version 6.0.0-Alpha (org.hibernate:hibernate-core -> org.hibernate.orm:hibernate-core).

@jonathanlermitage I like the ignoreList option, because some relocations should be postponed due to external factors (ex: javax. to jarkarta.). But I don't like it to be separated with commas. I would prefer to have multiple XML nodes (ex: exclude in https://maven.apache.org/enforcer/enforcer-rules/bannedDependencies.html)

jonathanlermitage commented 3 years ago

I think I agree with you, the main goal of this plugin is to detect artifacts relocations.
I understand that some projects may want to stick to an old library. This is probably an edge case (I may be wrong), but the ignore-list should fit this need (and yes, xml nodes will be more human-readable than commas ^_^). The ignore-list will exclude some artifacts, but it should not do this exclusion silently: we could add warning messages like "artifact xxx.yyy is relocated to aaa.bbb, but it has been filtered by ignore-list". As an example, this is the behavior of the OWASP plugin: it reminds you that you filtered some libraries. We could do the same thing.
Maybe we can find a better name than "ignore-list", I don't know. A single name probably can't cover the regular case (I am not ready to move from javax to jakarta yet) and the situation described by Delanym. But the result will be the same.

jonathanlermitage commented 3 years ago

ignore-list implemented. Please see the README file.
Will publish 1.4 version very soon.

delanym commented 2 years ago

@froque what does it mean to be "relocated"? The page https://mvnrepository.com/artifact/commons-collections/commons-collections doesn't talk about "re" location, it says the artifact was "moved". Relocating means, to me, to publish the same artifact for a second time under a different GAV number. This is not the case with the versions of the artifact known as commons-collections:commons-collections, nor is it the case for https://mvnrepository.com/artifact/org.apache.httpcomponents/httpclient which you will see does not have a note about moving to https://mvnrepository.com/artifact/org.apache.httpcomponents.client5/httpclient5 The determination is entirely arbitrary and up to the maintainer of this plugin, or you if you choose a different definitions file.

In practice, we all probably want to use the latest version of an artifact anyway, so correctness is sacrificed for utility.

froque commented 2 years ago

what does it mean to be "relocated"?

It means the Apache Commons project decided to change the Maven coordinates:

Change maven coordinates to "org.apache.commons.commons-collections4".

https://commons.apache.org/proper/commons-collections/release_4_0.html

The page https://mvnrepository.com/artifact/commons-collections/commons-collections doesn't talk about "re" location, it says the artifact was "moved". Relocating means, to me, to publish the same artifact for a second time under a different GAV number.

Please, be aware that mvnrepository.com is not part of Apache Maven or under Sonatype control. They do not share details how they do things.

You are correct that the oficial Maven guide to relocating talks about publish a new version under the old coordinates and a new version under the new coordinates. But very few projects have actual done it.
https://maven.apache.org/guides/mini/guide-relocation.html#releasing-the-next-version

The determination is entirely arbitrary and up to the maintainer of this plugin, or you if you choose a different definitions file.

Yes, the determination is entirely arbitrary for this plugin but also for mvnrepository.com. At least whith this plugin you can override the definitions file.

delanym commented 2 years ago

I would rather the plugin acted on any relocation tag in the current version of the dependency. That would be correct and not produce false positives, nor rely on arbitrary lists.

But then you might as well just add this functionality to the versions-maven-plugin (which everyone uses anyway).