You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/07/11 15:14:12 UTC
[GitHub] [maven] cstamas opened a new pull request, #765: Experiment: warn on ancient plugin
cstamas opened a new pull request, #765:
URL: https://github.com/apache/maven/pull/765
Warn if ancient plugin used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182963000
Ok so let me step back:
1. Goal is to ensure defined plugins are compatible with the maven version
2. Technically you cannot use maven-plugin API test like that to implement it, you need to ensure the API *used* by the plugin is available in current maven version
Since we care only about maven 4 there - guess for maven 3 the game is already played - we should probably add these metadata somewhere in 1. maven and 2. plugins (at build time with maven-plugin-plugin probably), this way the check is not a version check which is very error prone but an accurate check.
This means apache/maven build would dump a txt file in its resources with the supported plugin API and mojo would have in its own resources a META-INF/maven/whataver/used-api.txt. Comparing both we would get the compatibility for sure.
One trick can be to be able to configure a bit more this dump to enable optional parts of the mojo to be marked as being optional and theorically activated by this or this configuration (if <git> is not null class GitWhatever is used so the mojo api used in this class is needed in the validation for ex).
But until we have such a mecanism (computed build time so fast or at runtime with bytecode visiting - slower) then any work in that direction sounds like noise for end user.
Now the side note stays, it shouldn't be something enabled for all runs but only when user requests it, this is why I think it belongs to a plugin (versions was a good one but any other would work for me) and not maven core.
Also note that it is not very complicated for a plugin to validates its maven API dependencies when it is instantiated (loadClass, getMethod are more than sufficient) so it can just be about encouraging that.
Hope it is clearer.
Just to answer to your questions:
> AFAIK if you use maven 2 and the same surfacing API than with 3.1 then you don't want this warning
Means that the version is not a key point since maven 2 API and maven 3 overlap enough to have functional plugin with maven 2 so we must not warn for them.
> What about using this slower but more accurate implementation in versions-maven-plugin?
Was just referencing the bytecode based implementation, nothing linked to lifecycles.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183057949
> @cstamas exactly my point, you can upgrade maven to get new features (better jvm.config support, better CLI, optims, ...) but not the plugins and it must works without warnings until the API got broken AND the plugin uses this breaking change.
No, my point is quite the opposite :smile: IF you want to keep yourself up to date your Maven build, you need to consider your build as whole (maven + plugins, not just "bits" from it), not just the CLI, and, once you step over major version like 3.x to 4.x, you should not expect your 2.x plugins to work. And we should make this obvious to users as soon as possible.
Those who "can upgrade maven to get new features", but "cannot update the plugins"... maybe just need better tuition about Maven? Better docs? I mean, to me yet another contradiction: "update work tools" but "not plugins", while our doco is clear that plugins are "build tools" as well (from doco "Build plugins will be executed during the build ")....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183036354
> In short, IMHO we should nag users (and indirectly plugin devs as well) to keep up, especially if they want to sit on Maven 3.9 train that is taking them to Maven4.
This is where this all fails IMHO, it assumes there is always a new plugin version but in practise it is not true and what is probably worse is that it is not even needed technically. Would be really great to avoid to request releases without any change except a version upgrade + keep one of our strength which was a relatively high portability of the build accross versions compared to concurrents.
> Well, I believe they do not update their operating system, Java or anything, because it also "just works". Also, if it "just works" why would they go with Maven 3.9 (why would the go the fuss to upgrade maven in the first place?). These kind of statements are full of contradictions for me, so I never considered them.
This is exactly the opposite, they upgrade their work tools (maven , java, os etc) but the projects are under rules which are not "on your machine" so you can not always upgrade and the key point is previous one, we cant assume all plugin writers will release a plugin version per maven version - once again technically there is no reason we request that.
> And no, we will not break EVERYBODY, we will break (well, they will break themselves when upgrade to Maven4) those users who use Maven2 plugins, that's a huge difference: we have no resources to support compatibility across 2 major versions. Maven3 did support Maven2 plugins, just like Maven4 will support Maven3 plugins (but not Maven2). I think this is really fair expectation, but if not, stick with 3.8.6 as long as you want (the user I mean).
Yep but the current implementation does not respect that. I only have two points in my comments:
1. the warnings are not consistent and the heuristic used does not work so we should just go without another one,
2. hope it is not a default/all-the-time check (could be a daily like snapshot check) - I care more about seeing the warnings all the time than the perf for now until the algorithm used will be very slow but it is not yet the case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183234729
Let's move this discussion to ML.
---
Regarding this PR: is wrong, and the reason is that _provided scoped dependencies_ are not present in list I am checking (nor in DependencyNode root) as they are filtered out way before (buried in resolver).
Hence, is easy to trick out this check: use **latest** mpp and set **provided** scope for maven-plugin-api having version **2.2.1** :smile: and this code will not trigger (as provided scope is not seen).
We need a bit of different approach, and probably we need to grab plugin POM (not happening currently, only as "side effect" of resolving the plugin artifact) as that would be the simplest, and is already resolved (so is in local repo).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183264487
> @cstamas yes but real life, you know, will not happen ;)
NEVER give up! :smile:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182988405
> Now the side note stays, it shouldn't be something enabled for all runs but only when user requests it.
I disagree. Warnings on deprecated things should be (and have always been afaik) visible to the end user. The whole point of the deprecation is to alert the users so that he can upgrade to a more recent version. If those are not visible by default, I don't see the point of deprecating at all, we could just get rid of it and let users complain later ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #765:
URL: https://github.com/apache/maven/pull/765#discussion_r918084082
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -419,6 +421,32 @@ private void createPluginRealm( PluginDescriptor pluginDescriptor, MavenSession
pluginDescriptor.setClassRealm( pluginRealm );
pluginDescriptor.setArtifacts( pluginArtifacts );
+
+ try
+ {
+ GenericVersionScheme versionScheme = new GenericVersionScheme();
+ Version minimumVersion = versionScheme.parseVersion( "3.1.0" );
+ for ( Artifact artifact : pluginArtifacts )
+ {
+ if ( "org.apache.maven".equals( artifact.getGroupId() ) && "maven-plugin-api".equals(
+ artifact.getArtifactId() ) )
+ {
+ Version mavenPluginApiVersion = versionScheme.parseVersion( artifact.getVersion() );
+ if ( minimumVersion.compareTo( mavenPluginApiVersion ) > 0 )
+ {
+ logger.warn( "" );
+ logger.warn( "Plugin " + plugin.getId()
+ + " is old, please update to plugin supporting Maven 3.1+" );
Review Comment:
I'd suggest something like:
```The plugin uses an old version of the maven api, please update the plugin to support Maven 3.1+```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182977992
Well, just as you say, Maven3 has no API, hence, really no "easy" way to detect this "breakage" (as 3.x plugin could still declare dependency on maven-plugin-api:3.1 AND maven-compat:3.1 -- we may extend this check in this PR for that btw!), but one thing for sure: IF they use maven-plugin-api:2.x we KNOW it is either ancient plugin, or... If anyone MUST produce plugin that works with Maven2 AND Maven3, they should be aware they are missing the Maven4 train, and hence, I can assume they will stick with 3.8.6 for Maven3 (and who knows what version of Maven2) and it will not nag them. But, I bet we talk about minority here, and I guess as they do this, they already know what they are doing anyway.
Re metadata: If we start adding metadata, propagation of that metadata will take eons thru ecosystem (with 2.x plugins being still in use), so see my "slow rant" for that.
IMHO instead of some "lab" (scalpel) approach, yes, this is more like "root it out" (axe or hoe), just make it gone, kill it in root.
Hence, it is completely safe to assume if plugin:
* depends on plugin-api less than 3.1
* OR depends on maven-compat whatever 3.x version (to be added)
* => we deal with ancient plugin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183279304
Whole maven-compat is Maven2 compatibility module.
For mirrors you need MirrorSelector from RepositorySystemSession...
https://github.com/apache/maven-resolver/blob/master/maven-resolver-api/src/main/java/org/eclipse/aether/repository/MirrorSelector.java
If you can point me at code, may create a PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182947998
> What about using this slower but more accurate implementation in `versions-maven-plugin`? It is clearly the place it belongs to since this is not a check you want for each build but when you manage your versions _only_ IMHO.
Regarding "slower" implementation, we already tasted it, as
* users majority still use 2.x m-install-p/m-deploy-p (8 years) as we did not provide 3.x release of it
* users majority still use 2.x of surefire (as it is in M phase for 4 years)
* we keep finding codebase not touched for 5-10 years that needs cleanup
When Maven3 was released, there was an agreement to lift plugin versions to "align" it with Maven, so 3.x plugins were being made. We failed to deliver that even with "moist important" plugins (those that make Maven be Maven), the m-install-p and m-deploy-p. As you know, to run Maven2 plugin, you need maven-compat. That said, Maven4 will NOT support Maven2 plugins (we will not span 2 major versions, no resources for that), but Maven4 will support 3.x plugins. Hence, Maven 3.9.x -- as being in "corner" for upcoming Maven4 -- should start "herding" our users to keep up, as in this experiment, to at least move to Maven3 level 3.x plugins. Any plugin relying on maven-compat (and sadly this is not explicit thing, only way to "assume" it is use of plugin-api 2.x) is becoming critical in user build IF they want to keep up. If not, let them stick with old Java, old Maven, old Plugins....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183045029
I'd not want to prolong this discussion, but my 5 cents....
> they upgrade their work tools (maven , java, os etc)
For me, Maven Plugin _is a build tool_, as in contrast a dependency is not, and is completely okay to keep to "frozen". In other words, the fact that a plugin is defined in "sources" (well in POM), it does not mean it should have same rules applied as Java sources or dependencies.... But, that's me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182991878
@gnodet ok, what the user should do when it works and there is no new version because it just works? Keep using a tool with false positives? I really don't like this idea, in particular if by default - acceptable if not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #765:
URL: https://github.com/apache/maven/pull/765#discussion_r928173326
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -419,6 +424,29 @@ private void createPluginRealm( PluginDescriptor pluginDescriptor, MavenSession
pluginDescriptor.setClassRealm( pluginRealm );
pluginDescriptor.setArtifacts( pluginArtifacts );
+
+ try
+ {
+ GenericVersionScheme versionScheme = new GenericVersionScheme();
+ Version minimumVersion = versionScheme.parseVersion( "3.1.0" );
Review Comment:
Would it make sense to make is static final to avoid constant parsing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] kwin commented on a diff in pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #765:
URL: https://github.com/apache/maven/pull/765#discussion_r919117759
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -419,6 +421,32 @@ private void createPluginRealm( PluginDescriptor pluginDescriptor, MavenSession
pluginDescriptor.setClassRealm( pluginRealm );
pluginDescriptor.setArtifacts( pluginArtifacts );
+
+ try
+ {
+ GenericVersionScheme versionScheme = new GenericVersionScheme();
+ Version minimumVersion = versionScheme.parseVersion( "3.1.0" );
+ for ( Artifact artifact : pluginArtifacts )
+ {
+ if ( "org.apache.maven".equals( artifact.getGroupId() ) && "maven-plugin-api".equals(
+ artifact.getArtifactId() ) )
+ {
+ Version mavenPluginApiVersion = versionScheme.parseVersion( artifact.getVersion() );
+ if ( minimumVersion.compareTo( mavenPluginApiVersion ) > 0 )
+ {
+ logger.warn( "" );
+ logger.warn( "Plugin " + plugin.getId()
+ + " is old, please update to plugin supporting Maven 3.1+" );
Review Comment:
I think the message is targeted to the Maven user not the Plugin developer, so the advise to update the plugin to a newer version seems more reasonable to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183263633
@cstamas yes but real life, you know, will not happen ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183011964
@gnodet the issue is not us, we don't maintain a ton of plugins compared to what is used in builds, if we break everybody and/or make it very inconvenient to work with maven (~ prevent to upgrade to maven 4) I'm not sure we would get much adoption.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183226450
> > No, my point is quite the opposite smile IF you want to keep yourself up to date your Maven build, you need to consider your build as whole (maven + plugins, not just "bits" from it), not just the CLI, and, once you step over major version like 3.x to 4.x, you should not expect your 2.x plugins to work. And we should make this obvious to users as soon as possible.
>
> It is what is done (sadly or not) and while we don't really maintain minor branches I guess it will stay like that so must be taken into account whatever we think about it IMHO.
>
> > Those who "can upgrade maven to get new features", but "cannot update the plugins"... maybe just need better tuition about Maven?
>
> You assume again there is a new version of the plugin, keep in mind it is rarely true for anything outside maven ecosystem and in terms of metrics there are way more plugins outside org.apache.maven than inside.
>
> > Imho, it does not make sense to upgrade maven / java with the expectation that it will work forever without any migration. The plugins are definitely linked to a given maven version (range), even if maven tries to be compatible.
>
> Agree but expectation is to rarely break - and one again if a warning is printed it must not be random, current impl is equivalent to `if Math.random() > 0.5 then warning()`.
>
> > Those warnings are required in order to urge users and plugin developers to do this upgrade. That also means that we need to do the homework in our plugins too (hence the recent modifications on maven-install-plugin and maven-deploy-plugin), which should have been done a long time ago.
>
> Maybe an exit door until we get a saner ecosystem is to only do it for our own plugins and ignore (debug level?) it for external ones using the groupId?
I really don't get your point. If you don't show the deprecation, what will give plugin developers any incentive to upgrade ? The whole point of this PR is to _warn_ people that they should do it. It's been more than 10 years, they had ample time to do it. How much more time without any warning would you give them ? Another 10 years ?
I think if it has not been done so far, it's exactly because there was no warning...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183048070
@cstamas exactly my point, you can upgrade maven to get new features (better jvm.config support, better CLI, optims, ...) but not the plugins and it must works without warnings until the API got broken AND the plugin uses this breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183118399
> This is exactly the opposite, they upgrade their work tools (maven , java, os etc) but the projects are under rules which are not "on your machine" so you can not always upgrade and the key point is previous one, we cant assume all plugin writers will release a plugin version per maven version - once again technically there is no reason we request that.
Imho, it does not make sense to upgrade maven / java with the expectation that it will work forever without any migration. The plugins are definitely linked to a given maven version (range), even if maven tries to be compatible. The upgrade to maven >= 3.1 is a requirement to get rid of things that have been deprecated in 2008, so nearly 14 years ago. Those warnings are required in order to urge users and plugin developers to do this upgrade. That also means that we need to do the homework in our plugins too (hence the recent modifications on maven-install-plugin and maven-deploy-plugin), which should have been done a long time ago.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #765: Experiment: warn on ancient plugin
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1550469104
@cstamas is this PR still relevant with the recent 3.9.x changes ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183000717
> @gnodet ok, what the user should do when it works and there is no new version because it just works? Keep using a tool with false positives? I really don't like this idea, in particular if by default - acceptable if not.
This is not really acceptable, but we definitely need to get releases out real fast for the plugins that are provided by maven and which have not been upgraded or totally deprecated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183025980
> Sure but it is _not_ safe to say it will not work with current maven version. Best example is a plugin just adding some properties in maven project, logging some meta or generating some files from a preconfigured structure, it will run fine in most cases, so ok you have an old build of a plugin but you can't conclude much right? Last thing is "depends on" means "uses in its code" and not "has it in its dependencies" so still current impl is probably too optimistic and leads to too much false positive for end user IMHO.
It all started here: https://issues.apache.org/jira/browse/MPLUGIN-372
As there was a bug in mpp that prevented setting "maven bits" (so plugin-api, maven-core, etc) to provided scope, that resulted in maven downloading (other) maven versions just to exclude them from class path. And this annoyed me a LOT, and @rfscholte also told me he got feedback from users that were confused like "am using Maven X why did Maven download Maven Y" -- well, constituent artifacts of it. Is waste from whatever aspect you look at it (bandwidth, time, energy, warming oceans).
So, "old" plugin,to me it means several things:
* is build against "old" plugin-api (like 2.x)
* is built against maven-compat, so uses Maven2 compat layer despite is being built against Maven3
* most probably uses old tooling as well (new one emits warning during build to set proper scopes), so MOST PROBABLY will download whole maven as well.
In short, IMHO we should nag users (and indirectly plugin devs as well) to keep up, especially if they want to sit on Maven 3.9 train that is taking them to Maven4.
> what the user should do when it works and there is no new version because it just works?
Well, I believe they do not update their operating system, Java or anything, because it also "just works". Also, if it "just works" why would they go with Maven 3.9 (why would the go the fuss to upgrade maven in the first place?). These kind of statements are full of contradictions for me, so I never considered them.
> ...if we break everybody and/or make it very inconvenient to work with maven...
And no, we will not break EVERYBODY, we will break (well, they will break themselves when upgrade to Maven4) those users who use Maven2 plugins, that's a huge difference: we have no resources to support compatibility across 2 major versions. Maven3 did support Maven2 plugins, just like Maven4 will support Maven3 plugins (but not Maven2). I think this is really fair expectation, but if not, stick with 3.8.6 as long as you want (the user I mean).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] laeubi commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
laeubi commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183274479
Just wondering, is there a migration guide for those, e.g. using maven-compat? It would be good to include a link to this in the warning message, e.g. tycho uses maven-compat (or its dependencies) a lot but I often have no clue what is the alternative?
Example `org.apache.maven.repository.MirrorSelector` is used looking at the code:
```
/**
* Handles the selection of mirrors for repositories.
*
* @author Benjamin Bentmann
*/
public interface MirrorSelector
{
/**
* Determines the mirror for the specified repository.
*
* @param repository The repository to determine the mirror for, must not be {@code null}.
* @param mirrors The available mirrors, may be {@code null}.
* @return The mirror specification for the repository or {@code null} if no mirror matched.
*/
Mirror getMirror( ArtifactRepository repository, List<Mirror> mirrors );
}
```
1. **no deprecation**, so how should one know not to use it?
2. **no indication** what to use instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182989672
Here is an example of plugin that uses plugin-api 3.8.6 but DEPENDS on maven-compat (did not investigate why yet):
https://github.com/apache/karaf/blob/main/tooling/karaf-maven-plugin/pom.xml#L97-L106
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183257094
> I really don't get your point. If you don't show the deprecation, what will give plugin developers any incentive to upgrade ? The whole point of this PR is to warn people that they should do it. It's been more than 10 years, they had ample time to do it. How much more time without any warning would you give them ? Another 10 years ?
I think if it has not been done so far, it's exactly because there was no warning...
Ok so what you mean is that the PR should also check there is a more recent version of the plugin and this one is compatible, else the warning is just a noise user can't do anything about.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183259620
> Ok so what you mean is that the PR should also check there is a more recent version of the plugin and this one is compatible, else the warning is just a noise user can't do anything about.
This is very not true: they CAN do: they should nag the plugin author, just like we nag the user. This part was missing, and all happened silently and this is partially the reason nothing changed: everything was so silent :smile:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182330757
Hi,
Am I missing something or this PR makes any plugin not *compiled* with api >= 3.1.0 as deprecated?
Is it true? AFAIK if you use maven 2 and the same surfacing API than with 3.1 then you don't want this warning so it sounds to me that to implement this feature you should find another implementation path to avoid too much false positives.
One option can be to use the API signature and visit the plugin bytecode and check it uses deprecated/deleted API, this will be slower but at least the user reporting will be accurate cause right now it faces the end user with warning he can't do anything about - and worse being that the plugin developpers willing to do that will imply warnings on user side.
What about using this slower but more accurate implementation in `versions-maven-plugin`? It is clearly the place it belongs to since this is not a check you want for each build but when you manage your versions *only* IMHO.
Hope it makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1550816359
Is not relevant anymore, except one thing: current 3.9.x line does not check (like this PR does) is maven dep refd by plugin older than 3.1.0...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas closed pull request #765: Experiment: warn on ancient plugin
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas closed pull request #765: Experiment: warn on ancient plugin
URL: https://github.com/apache/maven/pull/765
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] laeubi commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
laeubi commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183281984
Tycho code: https://github.com/eclipse/tycho/
MirrorSelector specifically at
- https://github.com/eclipse/tycho/blob/master/tycho-core/src/main/java/org/eclipse/tycho/core/resolver/TychoMirrorSelector.java
- https://github.com/eclipse/tycho/blob/master/tycho-core/src/main/java/org/eclipse/tycho/osgi/configuration/RepositorySettingsConfigurator.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182990445
> If anyone MUST produce plugin that works with Maven2 AND Maven3, they should be aware they are missing the Maven4 train
Let see, I suspect we will need to ensure it works even if done with an extension enriching plugin depencencies on the fly, there is no real good way to break our ecosystem so should be something slow.
That said it does not help this topic to ensure a plugin can run on current maven version much since only way to be sure is to check the API surface which is used.
> Re metadata: If we start adding metadata, propagation of that metadata will take eons thru ecosystem (with 2.x plugins being still in use), so see my "slow rant" for that.
Agree, we can fallback on the bytecode visit in the meantime while this is not by default it should do the job.
> IMHO instead of some "lab" (scalpel) approach, yes, this is more like "root it out" (axe or hoe), just make it gone, kill it in root.
if so, let's just add in the plugin metadata the minimum maven version (potentially max?) and be it, no?
> Hence, it is completely safe to assume if plugin:
>
> * depends on plugin-api less than 3.1
> * OR depends on maven-compat whatever 3.x version (to be added)
> * => we deal with ancient plugin
Sure but it is *not* safe to say it will not work with current maven version. Best example is a plugin just adding some properties in maven project, logging some meta or generating some files from a preconfigured structure, it will run fine in most cases, so ok you have an old build of a plugin but you can't conclude much right? Last thing is "depends on" means "uses in its code" and not "has it in its dependencies" so still current impl is probably too optimistic and leads to too much false positive for end user IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1183191824
> No, my point is quite the opposite smile IF you want to keep yourself up to date your Maven build, you need to consider your build as whole (maven + plugins, not just "bits" from it), not just the CLI, and, once you step over major version like 3.x to 4.x, you should not expect your 2.x plugins to work. And we should make this obvious to users as soon as possible.
It is what is done (sadly or not) and while we don't really maintain minor branches I guess it will stay like that so must be taken into account whatever we think about it IMHO.
> Those who "can upgrade maven to get new features", but "cannot update the plugins"... maybe just need better tuition about Maven?
You assume again there is a new version of the plugin, keep in mind it is rarely true for anything outside maven ecosystem and in terms of metrics there are way more plugins outside org.apache.maven than inside.
> Imho, it does not make sense to upgrade maven / java with the expectation that it will work forever without any migration. The plugins are definitely linked to a given maven version (range), even if maven tries to be compatible.
Agree but expectation is to rarely break - and one again if a warning is printed it must not be random, current impl is equivalent to `if Math.random() > 0.5 then warning()`.
> Those warnings are required in order to urge users and plugin developers to do this upgrade. That also means that we need to do the homework in our plugins too (hence the recent modifications on maven-install-plugin and maven-deploy-plugin), which should have been done a long time ago.
Maybe an exit door until we get a saner ecosystem is to only do it for our own plugins and ignore (debug level?) it for external ones using the groupId?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1180556696
Example builds with this patch:
* commons-dbcp https://gist.github.com/cstamas/4c02cc32850915d51316e5a432d0ab1c
* maven-3.9.x https://gist.github.com/cstamas/a96d9e564e77bae5869ce25a77c8f3dc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on a diff in pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #765:
URL: https://github.com/apache/maven/pull/765#discussion_r919303086
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -419,6 +421,32 @@ private void createPluginRealm( PluginDescriptor pluginDescriptor, MavenSession
pluginDescriptor.setClassRealm( pluginRealm );
pluginDescriptor.setArtifacts( pluginArtifacts );
+
+ try
+ {
+ GenericVersionScheme versionScheme = new GenericVersionScheme();
+ Version minimumVersion = versionScheme.parseVersion( "3.1.0" );
+ for ( Artifact artifact : pluginArtifacts )
+ {
+ if ( "org.apache.maven".equals( artifact.getGroupId() ) && "maven-plugin-api".equals(
+ artifact.getArtifactId() ) )
+ {
+ Version mavenPluginApiVersion = versionScheme.parseVersion( artifact.getVersion() );
+ if ( minimumVersion.compareTo( mavenPluginApiVersion ) > 0 )
+ {
+ logger.warn( "" );
+ logger.warn( "Plugin " + plugin.getId()
+ + " is old, please update to plugin supporting Maven 3.1+" );
Review Comment:
Current message (on maven 3.9.x branch), modified to write out at end:
```
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache Maven 3.9.0-SNAPSHOT:
[INFO]
[INFO] Apache Maven ....................................... SUCCESS [ 2.175 s]
[INFO] Maven Model ........................................ SUCCESS [ 2.480 s]
[INFO] Maven Artifact ..................................... SUCCESS [ 0.698 s]
[INFO] Maven Plugin API ................................... SUCCESS [ 0.516 s]
[INFO] Maven Builder Support .............................. SUCCESS [ 0.207 s]
[INFO] Maven Model Builder ................................ SUCCESS [ 1.447 s]
[INFO] Maven Settings ..................................... SUCCESS [ 0.223 s]
[INFO] Maven Settings Builder ............................. SUCCESS [ 0.321 s]
[INFO] Maven Repository Metadata Model .................... SUCCESS [ 0.199 s]
[INFO] Maven Artifact Resolver Provider ................... SUCCESS [ 0.860 s]
[INFO] Maven Core ......................................... SUCCESS [ 3.458 s]
[INFO] Maven SLF4J Simple Provider ........................ SUCCESS [ 0.431 s]
[INFO] Maven Embedder ..................................... SUCCESS [ 0.845 s]
[INFO] Maven Compat ....................................... SUCCESS [ 1.557 s]
[INFO] Apache Maven Distribution .......................... SUCCESS [ 3.164 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 18.744 s
[INFO] Finished at: 2022-07-12T19:49:02+02:00
[INFO] ------------------------------------------------------------------------
[WARNING] Used pre-3.1 Maven plugins in session:
[WARNING] * org.apache.maven.plugins:maven-assembly-plugin:3.3.0
[WARNING] * org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0
[WARNING] * org.apache.maven.plugins:maven-failsafe-plugin:2.22.2
[WARNING] * org.apache.maven.plugins:maven-install-plugin:2.5.2
[WARNING] * org.apache.maven.plugins:maven-remote-resources-plugin:1.7.0
[WARNING] * org.apache.maven.plugins:maven-surefire-plugin:2.22.2
[WARNING] * org.apache.rat:apache-rat-plugin:0.13
[WARNING] * org.codehaus.mojo:build-helper-maven-plugin:1.12
[WARNING] * org.codehaus.mojo:buildnumber-maven-plugin:1.4
[WARNING] * org.codehaus.plexus:plexus-component-metadata:2.1.0
[WARNING] * org.eclipse.sisu:sisu-maven-plugin:0.3.5
[WARNING]
[WARNING] Total of 11
[WARNING] Please update to newer versions of these plugins.
[WARNING]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #765: Experiment: warn on ancient plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #765:
URL: https://github.com/apache/maven/pull/765#issuecomment-1182351356
> ...AFAIK if you use maven 2 and the same surfacing API than with 3.1 then you don't want this warning...
I completely lost it from here...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org