Runtime artifact's POM repos aren't propagated to resolve the deployment deps #3289
Conversation
…onding deployment deps
|
@gsmet could you please review it for me? From previous experience I don't think anybody else will :) Thanks. |
| } | ||
|
|
||
| public List<RemoteRepository> aggregateRepositories(List<RemoteRepository> dominant, List<RemoteRepository> recessive) { | ||
| return dominant.isEmpty() ? recessive : remoteRepoManager.aggregateRepositories(repoSession, dominant, recessive, false); |
There was a problem hiding this comment.
In which case do we have dominant and recessive repositories?
There was a problem hiding this comment.
That occurs naturally in any Maven project given that there is a hierarchy of POMs each of which may include repositories. Since during tests we, unfortunately, loose the Maven build context, we have to initialize repos the Maven way ourselves, taking the repos from the default settings and then merging them into the target project's repos. Repos from the project's POM will be dominating.
Another use-case is when we test/build an app from an external artifact (integration of external tests, for example). In that case, the project that integrates the external artifact is going to be dominating.
|
|
||
| private AppModel injectDeploymentDependencies(AppArtifact appArtifact, DependencyNode root, List<Dependency> managedDeps) throws AppModelResolverException { | ||
| DependencyNode resolvedDeps = mvn.resolveManagedDependencies(toAetherArtifact(appArtifact), | ||
| directMvnDeps, managedDeps, managedRepos, devmode ? new String[] { "test" } : new String[0]).getRoot(); |
There was a problem hiding this comment.
Sorry to bother you with my dumb questions but trying to understand what's going on to make a real review :).
Why exactly do we exclude the test scope when in dev mode and not in the other modes?
There was a problem hiding this comment.
When an app is compiled, every scope except test is available, which maps to our dev mode and that's how it's reflected in our code.
However, when the application is packaged provided and test scopes are filtered out by default. So, in that case, I am delegating to the default Maven resolver config that is taking care of that.
|
@aloubyansky @gsmet anything missing to get this PR merged ? |
|
@lburgazzoli no sorry, I am at a F2F so available as time permits. I just read Alexey's answer and it makes sense. Alexey, just wondering if maybe we should add a bit more comments for the special cases in this code. Because you have everything in mind but we don't :) (and even for you, in 4 months, it might get tricky). That being said, it can be done in a subsequent PR so let's not block this one. |
Fixes #3268