[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
GitHub user trask opened a pull request:

    https://github.com/apache/maven/pull/69

    MNG-5899 Reactor should use reduced dependency pom

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/trask/maven MNG-5899

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/maven/pull/69.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 #69
   
----
commit 1ca0deec41814815d82033645ca00fe6bbfdd813
Author: Trask Stalnaker <[hidden email]>
Date:   2015-10-01T22:09:41Z

    MNG-5899 Reactor should use reduced dependency pom

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
Github user ifedorenko commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-145722376
 
    -1
   
    The old behaviour allowed inconsistency between dependencies used to calculate project build order and project dependencies used during the build. It also resulted in reparsing reactor project pom.xml files multiple times during the build, which affected build performance for larger projects.
   



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
In reply to this post by Tibor17-2
Github user trask commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-145723525
 
    @ifedorenko, thanks for reviewing this.  Any thoughts what we should do with the downstream implication of this change?  https://issues.apache.org/jira/browse/MSHADE-206



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
In reply to this post by Tibor17-2
Github user ifedorenko commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-145727506
 
    I am not sure I fully understand the problem, but maven generally expects project dependencies to stay the same during the build. If you need to suppress certain storm-core dependencies from "leaking" into uber-jar projects, I think you should be able to simple mark those dependencies as optional=true (which really means "non-transitive").


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
In reply to this post by Tibor17-2
Github user trask commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-145730242
 
    The nice thing about how maven-shade-plugin's "dependency reduced pom" used to work, is that you could run mvn compile or mvn test from your parent module (and so maven shade plugin wouldn't kick in since it is bound to package phase), and transitive dependencies would work as normal.  And you could run mvn package or mvn install from your parent module, and maven shade plugin would kick in and expose the shaded jar and dependency reduced pom to downstream modules.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
In reply to this post by Tibor17-2
Github user revans2 commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-145854660
 
    @ifedorenko Without this kind of functionality using the shade plugin with multi-module projects is a huge pain, and would require us to use an external build system to coordinate the maven builds.  You have to fully build/install each shaded package independent of all other packages that depend on it.
   
    I understand that it is ugly to have the dependencies change as the build progresses.  I agree it really confuses things to run dependency:tree and see it be completely different from what is actually pulled in.  But it is not as bad as getting the wrong uber-jar that is missing things it needs.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven pull request: MNG-5899 Reactor should use reduced dependency...

Tibor17-2
In reply to this post by Tibor17-2
Github user aqueance commented on the pull request:

    https://github.com/apache/maven/pull/69#issuecomment-216595519
 
    In addition to the problems mentioned above, that [hack](https://github.com/apache/maven/blame/3533599e42a4a563abca33a69ed0f34c19815479/maven-aether-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java#L287) also breaks the transitive dependency collection functionality in general, because it employs a [Reactor-wide](https://github.com/apache/maven/blame/3533599e42a4a563abca33a69ed0f34c19815479/maven-core/src/main/java/org/apache/maven/ReactorReader.java#L69) [cache](https://github.com/apache/maven/blame/3533599e42a4a563abca33a69ed0f34c19815479/maven-core/src/main/java/org/apache/maven/ReactorReader.java#L145) of parsed POMs that fails to take into account the `RepositorySystemSession` contexts in which the cached POMs were parsed.
   
    A cache with a broken key scheme is not a cache, it's a source of unexpected behavior: if any `RepositorySystemSession` is at play other than the one used by the Reactor, collecting transitive dependencies now results in a dependency tree that is **practically random**, as it depends on:
   
      * what POMs have already been parsed by the Reactor during the build (this is why splitting the build into multiple phases works around the problems caused by this bug),
      * what other `RepositorySystemSession` instances have been used to parse and cache POMs so far,
      * what command line arguments were used to invoke Maven.
   
    I have a Maven plugin that computes transitive dependencies of the host project by cloning the `RepositorySystemSession` used by the reactor to set properties that activate build profiles, which in turn contain dependencies to be taken into account when computing a custom dependency tree.
   
    The API to do so is [public](http://wiki.eclipse.org/Aether) so one expects the functionality work, which it did until Maven 3.3 came along. Now because of that bug, the functionality stopped working reliably.
   
    Speed is important, sure, but correct functionality is probably even more so. So please remove that hack, and if its function is indeed necessary, then it needs to be replaced with a proper cache, one that takes the active `RepositorySystemSession` into account.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] maven issue #69: MNG-5899 Reactor should use reduced dependency pom

Tibor17-2
In reply to this post by Tibor17-2
Github user grahamtriggs commented on the issue:

    https://github.com/apache/maven/pull/69
 
    > Mutated pom.xml files must not invalidate original reactor ProjectDependencyGraph. More specifically, if the original graph allowed certain build order, the new graph must still allow the same order.
   
    That might be a practical limitation right now, but I wouldn't mind having a dynamic build order. The two things that should matter are that builds complete, and have the same final outcome. Dealing with the issues of being able "pull up" a dependency in the reactor, and knowing what can be built / is waiting on something else to be built might actually benefit scalability with parallel executions.
   
    Seems like there is a more important design question here - should a project, when built and installed to a repository, then depended on by a completely separate build behave the same when it is included in a reactor?
   
    If you can create an artifact, and a custom pom for install / deployment to a repository that differs from the project pom, then to my mind that should be what is seen by any module including it as a dependency, even in the same reactor.
   
    The concern is about adding new dependencies, and whilst that is technically possible, I'm not sure that it needs to be supported - it could have just been made a dependency of the project anyway.
   
    The real issue - particularly with the shade plugin - is that you want to embed things in an artifact, and not have other projects having to try and pull them in as dependencies. To be honest, it would actually be better if this was native to the pom, rather than part of the shade plugin, because then you could express what dependencies you want to embed, and this information would then be communicated to other projects depending on it. Then they would not only not pull in the transitive dependency for the embedded code, they would also be able to determine if the embedded version is compatible with their requirements.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] maven issue #69: MNG-5899 Reactor should use reduced dependency pom

Richard Sand
If the shaded artifact is built as a submodule with a different artifact
ID, then the shade plugin can specify its DRP as its main POM. It lets
other projects depend on the shaded artifact with only its remaining
(unshaded) dependencies.

I had tried to use the shade plugin along with the install & deploy
plugins so that I could build the main project, shaded artifact, and DRP
within a single POM, but was told that such practice was contrary to the
maven philosophy of all artifacts produced by a given POM have that POM
as its parent. So moving shade into a submodule with settings like this
was the only solution I could find that works:

   <createDependencyReducedPom>true</createDependencyReducedPom>
   
<keepDependenciesWithProvidedScope>false</keepDependenciesWithProvidedScope>
   <shadedArtifactAttached>false</shadedArtifactAttached>

Not sure if this adds anything to the thread but it worked for me

-Richard




------ Original Message ------
From: "grahamtriggs" <[hidden email]>
To: [hidden email]
Sent: 4/27/2017 1:30:24 PM
Subject: [GitHub] maven issue #69: MNG-5899 Reactor should use reduced
dependency pom

>Github user grahamtriggs commented on the issue:
>
>     https://github.com/apache/maven/pull/69
>
>     > Mutated pom.xml files must not invalidate original reactor
>ProjectDependencyGraph. More specifically, if the original graph
>allowed certain build order, the new graph must still allow the same
>order.
>
>     That might be a practical limitation right now, but I wouldn't mind
>having a dynamic build order. The two things that should matter are
>that builds complete, and have the same final outcome. Dealing with the
>issues of being able "pull up" a dependency in the reactor, and knowing
>what can be built / is waiting on something else to be built might
>actually benefit scalability with parallel executions.
>
>     Seems like there is a more important design question here - should
>a project, when built and installed to a repository, then depended on
>by a completely separate build behave the same when it is included in a
>reactor?
>
>     If you can create an artifact, and a custom pom for install /
>deployment to a repository that differs from the project pom, then to
>my mind that should be what is seen by any module including it as a
>dependency, even in the same reactor.
>
>     The concern is about adding new dependencies, and whilst that is
>technically possible, I'm not sure that it needs to be supported - it
>could have just been made a dependency of the project anyway.
>
>     The real issue - particularly with the shade plugin - is that you
>want to embed things in an artifact, and not have other projects having
>to try and pull them in as dependencies. To be honest, it would
>actually be better if this was native to the pom, rather than part of
>the shade plugin, because then you could express what dependencies you
>want to embed, and this information would then be communicated to other
>projects depending on it. Then they would not only not pull in the
>transitive dependency for the embedded code, they would also be able to
>determine if the embedded version is compatible with their
>requirements.
>
>
>---
>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 [hidden email] or file a JIRA
>ticket
>with INFRA.
>---
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: [hidden email]
>For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]