[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] maven issue #116: Fix jsr-330 injection of DefaultArtifactDescriptorReader
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
