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

Use regular Maven coordinates -when possible- for dependencies #443

Closed
mickaelistria opened this issue Dec 16, 2021 · 4 comments · Fixed by #473
Closed

Use regular Maven coordinates -when possible- for dependencies #443

mickaelistria opened this issue Dec 16, 2021 · 4 comments · Fixed by #473
Milestone

Comments

@mickaelistria
Copy link
Contributor

Currently, running mvn dependency:list for a Tycho module lists all dependencies under the form p2-installable-unit:org.domain.pluginId:version (eg p2-installable-unit:org.mockito.mockito-core:4.1.0). The scope is usually system and the absolute path to file is set.
In case where the dependency is known to have Maven coordinates, it would be better to reuse the coordinates of this dependency to showcase it's the "official" artifact from Maven repo. So in case of Mockito, it would become org.mockito:mockito-core:4.1.0. p2 metadata are expected to contain the necessary info to map a p2 artifact/unit back to the Maven coordinates they're generated from, those data should then be used in P2ResolverImpl when calling result.addArtifact to not hardcode usage of ArtifactType.TYPE_INSTALLABLE_UNIT and let Maven coordinates be used in place.
I expect that as long as the scope remains system and path is hardcoded, this shouldn't be too disruptive.

@akurtakov
Copy link
Member

It's closely related to eclipse-dash/dash-licenses#125 and this might fix the dash-licenses bug even.

@mickaelistria
Copy link
Contributor Author

One hotspot would be MavenDependencyInject#injectMavenDependencies()

@laeubi
Copy link
Member

laeubi commented Dec 18, 2021

Just one thing that came into my mind, given we have a repository of type p2 specified, it should be possible to even resolve p2-installable-unit:<unit>:<version> in a regular maven build using a core extension in .mvn folder. That would then download the unit from the repository and set the appropriate path before the actual build start.

@mickaelistria
Copy link
Contributor Author

Some incremental work can be done to ease implementation of this feature:

  • Remove P2ResolutionResults.Entry and use ArtifactDescriptor instead in P2ResolutionResult
  • Add a getMavenCoordinates() method, returning a GAV to ArtifactDescriptor
  • Make P2DependencyResolver set the Maven coordinates for given ArtifactDescriptor according to p2 metadata
  • Make MavenDependencyInject#injectMavenDependencies() consume the GAV when available to populate dependencies instead of using the ArtifactKey (which is p2 more than Maven).

laeubi added a commit to laeubi/tycho that referenced this issue Jan 3, 2022
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/tycho that referenced this issue Jan 3, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jan 3, 2022
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
github-actions bot pushed a commit to laeubi/tycho that referenced this issue Jan 4, 2022
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
github-actions bot pushed a commit to laeubi/tycho that referenced this issue Jan 4, 2022
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/tycho that referenced this issue Jan 5, 2022
Recover maven nature of an artifact by inspecting its attached IU

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
mickaelistria pushed a commit that referenced this issue Jan 5, 2022
Recover maven nature of an artifact by inspecting its attached IU

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
mickaelistria added a commit to mickaelistria/tycho that referenced this issue Jan 5, 2022
mickaelistria added a commit to mickaelistria/tycho that referenced this issue Jan 5, 2022
mickaelistria added a commit that referenced this issue Jan 5, 2022
@laeubi laeubi added this to the 2.7 milestone Feb 13, 2022
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

Successfully merging a pull request may close this issue.

3 participants