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

Unpacking dependencies duplicates files when two different versions of a dependency exist in the dependency tree #134

Closed
joh-ivu opened this issue Mar 20, 2024 · 3 comments · Fixed by #148, #149 or #151
Assignees
Labels
bug Something isn't working

Comments

@joh-ivu
Copy link
Contributor

joh-ivu commented Mar 20, 2024

In a company-internal project I have the following setup:

  • The FooBar project shall publish some .proto files
  • Those protos depend on other protos coming from Maven dependencies. There are three dependencies in total:
    • com.google.api.grpc:proto-google-common-protos (a jar with both .class and .proto files)
    • my.company:stuff (a zip with .proto files)
    • my.company:things (a zip with .proto files)
  • my.company:things in turn depends on stuff and proto-google-common-protos. On of course, all of them depend on the well known types like StringValue. This way, proto-google-common-protos and protobuf-java are present twice in the dependency tree, but as is so often the case in two different versions.

The unpacking of dependencies does not seem to take this into account leading to duplicate files:

2024-03-20 10_39_15-Window

@ascopes
Copy link
Owner

ascopes commented Mar 20, 2024

At the moment, the code doesn't deal with differentiating between multiple versions of the same dependency, which appears to be the issue here. I could probably deduplicate based upon the group ID and artifact ID though to avoid this. I'd need to take a look at how the dependency resolver API exposes this detail. When I get time I will do that though (I'm not going to have much time for the next couple of weeks so it might have to wait until then).

A possible workaround for now could be to explicitly exclude the dependencies you don't want in the dependencies block, e.g.

<dependencies>
  <dependency>
    <groupId>foo</groupId>
    <artifactId>bar</artifactId>
    
    <exclusions>
      <exclusion>
        <groupId>dependency</groupId>
        <artifactId>to-ignore-here</artifactId>
      </exclusion>
    </exclusions>
  </dependency>
</dependencies>

That way, it should be omitted from the dependency tree that the plugin can see.

That aside, I'll need to have a look into exactly how Maven deals with this at the moment for regular dependencies and see if I can achieve a similar thing. Right now we're just calling the dependency tooling that is used by maven-dependency-plugin to resolve dependencies (see https://github.com/ascopes/protobuf-maven-plugin/blob/main/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/MavenDependencyPathResolver.java#L115), but GH-101 tracks work I'd like to do to move onto the resolution API exposed by Maven directly when I get some time, so this may be an improvement for when I do that if I cannot achieve what I need for the current API.

@ascopes ascopes added bug Something isn't working help wanted Looking for help from someone with expertise or time feature request and removed help wanted Looking for help from someone with expertise or time labels Mar 20, 2024
ascopes added a commit that referenced this issue Apr 4, 2024
@ascopes
Copy link
Owner

ascopes commented Apr 5, 2024

@joh-ivu I've just made an unrelated change that updates how dependency resolution works. One of the things I changed was that any Maven-based project dependencies will now make use of the original Maven project dependency resolution rather than re-resolving all dependencies twice.

It would appear that a side effect of doing this is that it may now be deduplicating these dependencies in the way that you would expect. The test case for GH-132 which resulted in duplicate dependencies being output now appears to only be taking in one copy of protobuf-java despite depending on 4 modules that all depend on it transitively.

Would you mind retesting using the latest build on main and see if it now follows your expectations? If it does not, I may need to get some more details from you about reproducing this issue. If it does fix it, then this will be fixed in v1.2.0 which will be released once I've addressed your other issue at GH-135.

If you need to build this locally to test against another project, you can just do

$ git clone git@github.com:ascopes/protobuf-maven-plugin
$ cd protobuf-maven-plugin
$ ./mvnw clean install -Dmaven.test.skip -Dinvoker.skip

...then use 1.2.0-SNAPSHOT as the plugin version in your project.

Sorry it has taken so long to address this. Thanks for the patience.

@ascopes ascopes self-assigned this Apr 5, 2024
ascopes added a commit that referenced this issue Apr 5, 2024
GH-134: Write test to check for duplicated dependencies when versions differ
@ascopes ascopes reopened this Apr 5, 2024
@ascopes
Copy link
Owner

ascopes commented Apr 7, 2024

I have looked into this further and I believe that this is fixed by the changes I've added to main. If you see any further issues aside from this, please can you reraise a new issue or ping me here and I shall reopen this issue!

@ascopes ascopes closed this as completed Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment