Skip to content
This repository has been archived by the owner on Dec 23, 2021. It is now read-only.

Fix module deduplication #70

Merged
merged 1 commit into from
Oct 16, 2019
Merged

Fix module deduplication #70

merged 1 commit into from
Oct 16, 2019

Conversation

TimMoore
Copy link

- Classifiers may have values other than "native".
- Artifact.type is a String, not an Option[String].
- Fill in missing tests.
@dwijnand
Copy link
Member

dwijnand commented Oct 16, 2019

I'm concerned this is making it overly lax. I'd be happy to add linux-x86_64 as a classifier that gets ignored, but I think it's safer to blow up with a "Multiple elements for the same key" exception than to silently choose to ignore anything with a classifier.

But I'm open to being convinced.

@TimMoore
Copy link
Author

The classifier is defined by the artifact. It has no intrinsic meaning. We would potentially have to keep adding exceptions case-by-case.

@TimMoore
Copy link
Author

If it does blow up, how else would we handle it, other than by creating a new exception here?

@dwijnand
Copy link
Member

Classifiers have meaning, otherwise why would they exist? They come from libraryDependencies. I'm not sure how we'd handle it, but we'd have to think about it.

If we want to add a way (e.g. a callback thingy) to allow downstream builds to handle it, then that would be fine for me.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed my mind. The metadata must flow. And we'll pick up the pieces if it ends up being a mistake.

@dwijnand dwijnand merged commit 0b05c1c into lightbend:master Oct 16, 2019
@TimMoore
Copy link
Author

Thanks, @dwijnand. What I meant by that is that the meaning is ascribed by the author of the library that uses it. The string "native" isn't magic, it's just what the jffi artifact happens to use. And netty-transport-native-epoll happens to use linux-x86_64. Other artifacts could use other strings, they're arbitrary identifiers as far as Maven and sbt are concerned (with the exceptions of tests and sources which are built-in conventions).

It would be better if you could include all classifiers, because indeed these might contain different code, but I assume that would require changes on Whitesource's side to track libraries at that level of granularity.

@TimMoore TimMoore deleted the fix-module-deduplication branch October 16, 2019 10:17
@dwijnand
Copy link
Member

It might cause a problem, but as far as their programmatic API goes you can set the classifier - which is why it's even being touched here.

So we could, if we wanted to, get this plugin out of the business of trying to sanitise the data and handle dedup within Whitesource. But that's not necessary for now.

dwijnand added a commit to dwijnand/zinc that referenced this pull request May 1, 2020
Without this Travis CI was failing:
https://travis-ci.org/github/sbt/zinc/jobs/671727552

    [error] Caused by: java.lang.RuntimeException: Multiple elements for the same key (org.scala-lang.modules,scala-xml_2.12):
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	at scala.sys.package$.error(package.scala:30)
    [error] 	at sbtwhitesource.package$KeyByAndMergeSyntax$.$anonfun$keyByAndMerge$2(package.scala:26)

This brings in the following 2 fixes:

* lightbend/sbt-whitesource#68
* lightbend/sbt-whitesource#70
dwijnand added a commit to ijuma/zinc that referenced this pull request May 1, 2020
Without this Travis CI was failing:
https://travis-ci.org/github/sbt/zinc/jobs/671727552

    [error] Caused by: java.lang.RuntimeException: Multiple elements for the same key (org.scala-lang.modules,scala-xml_2.12):
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	at scala.sys.package$.error(package.scala:30)
    [error] 	at sbtwhitesource.package$KeyByAndMergeSyntax$.$anonfun$keyByAndMerge$2(package.scala:26)

This brings in the following 2 fixes:

* lightbend/sbt-whitesource#68
* lightbend/sbt-whitesource#70
dwijnand added a commit to ijuma/zinc that referenced this pull request May 1, 2020
Without this Travis CI was failing:
https://travis-ci.org/github/sbt/zinc/jobs/671727552

    [error] Caused by: java.lang.RuntimeException: Multiple elements for the same key (org.scala-lang.modules,scala-xml_2.12):
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	ModuleInfo(org.scala-lang.modules,scala-xml_2.12,1.2.0,Some((Artifact(scala-xml_2.12, bundle, jar, None, Vector(), Some(https://repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar), Map(), None, false),/home/travis/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/1.2.0/scala-xml_2.12-1.2.0.jar)))
    [error] 	at scala.sys.package$.error(package.scala:30)
    [error] 	at sbtwhitesource.package$KeyByAndMergeSyntax$.$anonfun$keyByAndMerge$2(package.scala:26)

This brings in the following 2 fixes:

* lightbend/sbt-whitesource#68
* lightbend/sbt-whitesource#70
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants