You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by jdillon <gi...@git.apache.org> on 2017/05/14 19:46:53 UTC
[GitHub] maven pull request #116: Fix jsr-330 injection of DefaultArtifactDescriptorR...
GitHub user jdillon opened a pull request:
https://github.com/apache/maven/pull/116
Fix jsr-330 injection of DefaultArtifactDescriptorReader
... which is not setting the VersionRangeResolver; cause NPE when use w/o plexus requirement injection
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jdillon/maven DefaultArtifactDescriptorReader-injection-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/maven/pull/116.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #116
----
commit 0c3f3a42262c5692742abc73042a1deece0898e0
Author: Jason Dillon <ja...@planet57.com>
Date: 2017-05-14T19:45:11Z
Fix jsr-330 injection of DefaultArtifactDescriptorReader; which was not setting the VersionRangeResolver
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
Igor's change is the more correct/ideal change to avoid future mismatch between plexus-annotation configuration and JSR-330 annotations. The problem is that plexus @Requirement injects directly to the field, but JSR-330 uses @Inject on the ctor, and the ctor impl ignores one of the parameters and doesn't fully setup the object properly. Its generally bad to have a mix of these annotations, as the lifecycle between plexus and jsr-330 is slightly different, but if you do mix them, you have to ensure that both means wire up the object correctly.
My change simply fixes the bug, so that the versionRangeResolver component, which is required for the DefaultArtifactDescriptorReader impl to function properly actually gets set.
My recommendation is you merge this change as its very simple and easy to comprehend (or at least I thought) and then consider merging Igor's change as this is more ideal for the future of the library.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@ifedorenko That's the only way w/o INFRA: commit message "This closes #xy". You cannot manually close via website. That's what I was trying to say.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by ifedorenko <gi...@git.apache.org>.
Github user ifedorenko commented on the issue:
https://github.com/apache/maven/pull/116
Opened https://issues.apache.org/jira/browse/MNG-6233. If you any concerns or suggestions, I suggest we continue the discussion there.
@jdillon I can't close this pull request
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
closing, https://github.com/apache/maven/commit/66fc74d6296ea0a33f8a9712dc5ed5eb3affd529 provides more comprehensive adjustments for JSR-330 annotations
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@ifedorenko Closing PRs requires a ticket with INFRA on JIRA.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
@ifedorenko ftr I can close this, so but i'm confused about @michael-o comment that your change (I think that is what he meant) wasn't suitable for a patch release.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
Thanks for the explanation. So it is not a prerequisite directly. I'd still insist for an JIRA because I need another committer to second this and have this in our release notes. Do you have a simple usecase to turn into an IT? I am fine with merging your change, looks pretty reasonable to me. Clearly a bug.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@jdillon Thank you for the contribution anyway!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
Hey folks can we get some movement on this and/or https://github.com/apache/maven/compare/MNG-6233_maven-resolver-provider-jsr330
ATM its broken, either of these fixes the issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by ifedorenko <gi...@git.apache.org>.
Github user ifedorenko commented on the issue:
https://github.com/apache/maven/pull/116
Wow. That's backwards. I wonder what will happen if I push my change with github's magic "fixes 116" pseudo comment. Guess there is one way to find out :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
the plexus @Requirement injection is probably why this works at all in Maven, though w/o using maven-resolver-provider in a pure guice/sisu environment, this fails and causes NPE:
```
java.lang.NullPointerException
at org.apache.maven.repository.internal.DefaultModelResolver.resolveModel (DefaultModelResolver.java:198)
at org.apache.maven.model.building.DefaultModelBuilder.readParentExternally (DefaultModelBuilder.java:1051)
at org.apache.maven.model.building.DefaultModelBuilder.readParent (DefaultModelBuilder.java:829)
at org.apache.maven.model.building.DefaultModelBuilder.build (DefaultModelBuilder.java:331)
at org.apache.maven.repository.internal.DefaultArtifactDescriptorReader.loadPom (DefaultArtifactDescriptorReader.java:321)
at org.apache.maven.repository.internal.DefaultArtifactDescriptorReader.readArtifactDescriptor (DefaultArtifactDescriptorReader.java:199)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.resolveCachedArtifactDescriptor (DefaultDependencyCollector.java:544)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.getArtifactDescriptorResult (DefaultDependencyCollector.java:528)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.processDependency (DefaultDependencyCollector.java:418)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.processDependency (DefaultDependencyCollector.java:372)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.process (DefaultDependencyCollector.java:360)
at org.eclipse.aether.internal.impl.DefaultDependencyCollector.collectDependencies (DefaultDependencyCollector.java:263)
at org.eclipse.aether.internal.impl.DefaultRepositorySystem.collectDependencies (DefaultRepositorySystem.java:325)
...
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request #116: Fix jsr-330 injection of DefaultArtifactDescriptorR...
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon closed the pull request at:
https://github.com/apache/maven/pull/116
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
Igor already created https://issues.apache.org/jira/browse/MNG-6233 but if you really need another issue I can file one just to fix this specific problem.
Seems a bit odd though that you want an IT to apply a clear coding mistake due to the ctor parameter being ignored and not setting the component causing NPE when used. :-(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@jdillon Igor merged his PR contained your fix. This PR is obsolete now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
@ifedorenko howdy, i'm fine to wait if you are gonna adjust to jsr330 that will be better generally for the project anyways IMO; i'm just trying to learn how to use the api and can live with this change locally for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
Just to be clear, Igor's change is s prerequisite to your issue? If so, we need two tickets. The IT assures that nothing will break your fix -- no more, no less.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by jdillon <gi...@git.apache.org>.
Github user jdillon commented on the issue:
https://github.com/apache/maven/pull/116
@ifedorenko can close this if you like, wasn't sure what you mean about "a JIRA either way". If we close this and go with your changes (which if you have that mostly done for merge soon is what I would suggest over this). For now I'm assuming you'll merge you changes and file a JIRA, but if you need a JIRA specifically about this bug I can file it if you need.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@jdillon Such a change is not suitable for a patch version in my opinion. Please file a JIRA issue for your local branch and push a branch for that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by michael-o <gi...@git.apache.org>.
Github user michael-o commented on the issue:
https://github.com/apache/maven/pull/116
@jdillon I'd like to merge this but need a JIRA issue. At best, an IT too. Can you provide a simple one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
Posted by ifedorenko <gi...@git.apache.org>.
Github user ifedorenko commented on the issue:
https://github.com/apache/maven/pull/116
I have a commit on a local branch that fully converts maven-resolver-provider to jsr330, I can push that to master if you can wait few days. Either way we'll need a JIRA to track the changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org