Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<optional> of transitive dependencies when <flattenMavenMode> is "all" #160

Closed
yangnuoyu opened this issue Jun 15, 2020 · 5 comments
Closed

Comments

@yangnuoyu
Copy link
Contributor

When I tried to flatten a pom with dependencies whose is "true" and set flattenDependencyMode == all, I got a flattened pom which parsed the transitive dependencies but with the as "false". I am wondering if it is a potential problem.

The pom we want to flatten contains dependencies:

  <dependencies>
    <dependency>
      <groupId>org.codehaus.mojo.flatten.its</groupId>
      <artifactId>core</artifactId>
      <version>3.2.1</version>
      <optional>true</optional>
    </dependency>
  </dependencies>

The core has transitive dependency:

  <dependencies>
    <dependency>
      <groupId>org.codehaus.mojo.flatten.its</groupId>
      <artifactId>dep</artifactId>
      <version>3.2.1</version>
    </dependency>
  </dependencies>

What I got in the flattened pom:

  <dependencies>
    <dependency>
      <groupId>org.codehaus.mojo.flatten.its</groupId>
      <artifactId>core</artifactId>
      <version>3.2.1</version>
      <scope>compile</scope>
      <exclusions>
        <exclusion>
          <artifactId>dep</artifactId>
          <groupId>org.codehaus.mojo.flatten.its</groupId>
        </exclusion>
      </exclusions>
      <optional>true</optional>
    </dependency>
    <dependency>
      <groupId>org.codehaus.mojo.flatten.its</groupId>
      <artifactId>dep</artifactId>
      <version>3.2.1</version>
      <scope>compile</scope>
      <optional>false</optional>
    </dependency>
  </dependencies>
@stephaniewang526
Copy link
Contributor

stephaniewang526 commented Jun 17, 2020

This is a valid issue -- as observed in java-bigtable when we try to add flatten plugin with flattenDependencyMode == all in the google-cloud-bigtable-emulator module. Full emulator module pom can be found here.

flattened pom shows:

<dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.13</version>
      <scope>compile</scope>
      <optional>true</optional>
      <exclusions>
        <exclusion>
          <artifactId>hamcrest-core</artifactId>
          <groupId>org.hamcrest</groupId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>org.hamcrest</groupId>
      <artifactId>hamcrest-core</artifactId>
      <version>1.3</version>
      <scope>compile</scope>
      <optional>false</optional> -- INCORRECTLY SET AS FALSE
    </dependency>
  </dependencies>

As a result, mvn dependency:list generates a list of all (both direct and transitive) dependencies as below:

[INFO]    com.google.api:api-common:jar:1.9.0:compile
[INFO]    com.google.auto.value:auto-value-annotations:jar:1.7.2:compile
[INFO]    com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]    com.google.errorprone:error_prone_annotations:jar:2.3.4:compile
[INFO]    com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]    com.google.guava:guava:jar:29.0-android:compile
[INFO]    com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO]    com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]    io.grpc:grpc-api:jar:1.29.0:provided
[INFO]    io.grpc:grpc-context:jar:1.29.0:provided
[INFO]    javax.annotation:javax.annotation-api:jar:1.3.2:compile
**[INFO]    junit:junit:jar:4.13:compile (optional)** 
[INFO]    org.checkerframework:checker-compat-qual:jar:2.5.5:compile
[INFO]    org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:provided
**[INFO]    org.hamcrest:hamcrest-core:jar:1.3:compile (optional)** 

The flatten plugin, with flattenDependencyMode == all is generating below instead:

[INFO]    com.google.api:api-common:jar:1.9.0:compile
[INFO]    com.google.auto.value:auto-value-annotations:jar:1.7.2:compile
[INFO]    com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]    com.google.errorprone:error_prone_annotations:jar:2.3.4:compile
[INFO]    com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]    com.google.guava:guava:jar:29.0-android:compile
[INFO]    com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO]    com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]    io.grpc:grpc-api:jar:1.29.0:provided
[INFO]    io.grpc:grpc-context:jar:1.29.0:provided
[INFO]    javax.annotation:javax.annotation-api:jar:1.3.2:compile
**[INFO]    junit:junit:jar:4.13:compile**
[INFO]    org.checkerframework:checker-compat-qual:jar:2.5.5:compile
[INFO]    org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:provided
**[INFO]    org.hamcrest:hamcrest-core:jar:1.3:compile**

Notice that "(optional)" is incorrectly set for the transitive dependency (org.hamcrest:hamcrest-core:jar:1.3).

This should be addressed before we can implement flatten plugin in GCP cloud client modules that contain <optional>true</optional> dependencies.

@saturnism @olamy FYI

@saturnism
Copy link
Contributor

saturnism commented Jun 17, 2020

it looks like the issue is similar to <scope>provided</scope> is being carried through as well, e.g.,

consumer -> lib A -> lib B -> lib C (optional)

before flattening, from the perspective of lib A, it's lib A -> lib B and the optional doesn't show up as a transitive.

after flattening, it becomes lib A -> lib B and lib C (optional), because lib C has been pulled up as a direct, but retain the optional/provided scopes.

from the consumer perspective of either modes, it's consumer -> lib A -> lib B, and consumer will drop optional/provided dep from transitive deps.

but these extra information is unnecessary can prob be removed in the final flattened pom.

@saturnism
Copy link
Contributor

saturnism commented Jun 17, 2020

@stephaniewang526 pointed out the bigger issue I misread from the initial description:

consumer -> lib A -> lib B (optional) -> lib C

but from perspective of lib A, it's lib A -> lib B (optional) and lib C. so lib C should also be removed from the transitive deps rather than being pulled up.

from consumer's perspective: consumer -> lib A -> lib C, which is incorrect.

@yangnuoyu
Copy link
Contributor Author

First, we need to make sure that we use dependency-plugin:3.1.2 to generate the dependency list, otherwise there will not show "(optional)" for optional dependencies in the pom.

Second, for lib A -> lib B (optional) -> lib C, if checking the dependency:tree or dependency:list of A, we can find out that lib C will be still in the class path of A but set as "optional", i.e. lib A -> lib B (optional) -> lib C(optional). So I think we need to keep the lib C in the flattened A, i.e. lib A (flattened) -> lib B (optional) and lib C (optional).

yangnuoyu added a commit to yangnuoyu/flatten-maven-plugin that referenced this issue Jun 30, 2020
… its transitive dependencies and set them as optional
@stephaniewang526
Copy link
Contributor

stephaniewang526 commented Jun 30, 2020

Looks like using the correct version of maven-dependency-plugin (3.1.2) is key to showing (optional) in dependency tree. We could add a section in the flattened pom by configuring to ensure that we have this correctly managed from java-shared-config (like in the original pom). @saturnism any concerns?

@olamy olamy closed this as completed in 45eb5df Jul 16, 2020
olamy pushed a commit that referenced this issue May 3, 2022
…nsitive dependencies and set them as optional (#164)

* fixes #160: when the direct dependency is optional, flattened its transitive dependencies and set them as optional

* remove optional dependency subtree

* return false in visitor when node is not included
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants