Override scala organization and version transitively at the Ivy level by milessabin · Pull Request #2634 · sbt/sbt

milessabin · 2016-05-30T14:24:47Z

This is a first cut at a PR intended to elicit feedback.

The primary motivation for this change is to allow an alternative Scala organization to be used in builds which have dependencies which themselves have direct dependencies on Scala distribution artefacts (eg. scala-reflect, scala-compiler). As well as supporting alternative Scala organizations I believe that this change also addresses #2286 and some of the concerns raised in the discussion on that ticket.

The change is to use an Ivy DependencyDescriptorMediator to rewrite all direct and transitive dependencies on scala-library, scala-compiler, scala-reflect, scala-actors, and scalap to have the same organization (either specified by the scalaOrganization setting or defaulting to org.scala-lang) and version (as specified by scalaVersion).

Issues to be resolved before merging,

Although this behaviour should be the default, it should probably be made configurable. How should such an option be exposed? If an attribute of UpdateOptions, how would that be made available to IvyScala?
How should this be tested? Locally I have been able to verify that it behaves as intended by running the included scripted test. However this relies on a local build of Scala with the organization set to "com.milessabin". Should this be done via a mock repository? Is there an example that I could base tests on in the tree?

lightbend-cla-validator · 2016-05-30T14:24:48Z

Hi @milessabin,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://www.lightbend.com/contribute/cla

fommil · 2016-05-30T14:32:56Z

build.sbt

 def buildLevelSettings: Seq[Setting[_]] = inThisBuild(Seq(
  organization := "org.scala-sbt",
-  version := "0.13.12-SNAPSHOT",
+  version := "0.13.12-devel",


you probably didn't mean to commit this

This is a PoC for comment ... Yolo etc. ...

ghost · 2016-05-30T15:21:11Z

I have a general comment, that moves this PR beyond its current scope, but I'll mention it in any case. In this PR, functionality is added to override the groupid for some "known" modules - the scala-lang ones. But it could be that other builds may want to do the same on their "known" modules.

So would it be useful to have a general OverrideOrganizationMediator, and then overrideScalaVersion calls that?

milessabin · 2016-05-30T15:47:52Z

@inthenow I think that would be extremely useful, but I think it deserves a separate PR ... most likely it would use the same underlying mechanisms.

ghost · 2016-05-30T15:55:40Z

@milessabin Excellent - I think two PR's is best as well, with this one setting "the scene". Just thought I should mention it sooner rather than later

milessabin · 2016-05-30T16:02:09Z

@inthenow 👍 ... it was at the back of my mind as well :-)

dwijnand · 2016-05-30T17:38:26Z

Nice and simple, nice.

eed3si9n · 2016-05-30T18:46:25Z

ivy/src/main/scala/sbt/IvyScala.scala

      excludeScalaJars(module, check.configurations)
-    if (check.overrideScalaVersion)
-      overrideScalaVersion(module, check.scalaFullVersion)
+    if (true || check.overrideScalaVersion)


If an attribute of UpdateOptions, how would that be made available to IvyScala?

Maybe we can use this overrideScalaVersion, which is where you're hooking your code anyway.
I'm ok with flipping the default value of this flag to true.

lightbend-cla-validator · 2016-05-31T09:26:31Z

Hi @milessabin,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://www.lightbend.com/contribute/cla

milessabin · 2016-05-31T09:28:29Z

main/src/main/scala/sbt/Defaults.scala

    updateOptions := (updateOptions in Global).value,
+    ivyScala <<= ivyScala or (scalaHome, updateOptions in Global, scalaVersion in update, scalaBinaryVersion in update, scalaOrganization, sbtPlugin) { (sh, uo, fv, bv, so, plugin) =>
+      Some(new IvyScala(fv, bv, Nil, filterImplicit = false, checkExplicit = true, overrideScalaVersion = plugin | uo.overrideScalaVersion, scalaOrganization = so))
+    },


I'd welcome review on this change ... is this the right way to pass the new UpdateOptions field through to IvyScala?

I'm actually ok with just putting true where it says plugin | uo.overrideScalaVersion, and roll back the change you made in UpdateOptions.

In that case there wouldn't be a mechanism to escape back to the current no-override behaviour. I have no problem with that, and it means that we could get rid of the conditional in checkModule ... if you're sure that's what you want I'm happy to do that.

The incantation to get the old school behavior back would be:

ivyScala := { ivyScala.value map {_.copy(overrideScalaVersion = sbtPlugin.value)} }

Ah ... gotcha. Right, I'll do that.

lightbend-cla-validator · 2016-05-31T14:19:37Z

Hi @milessabin,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://www.lightbend.com/contribute/cla

milessabin · 2016-06-01T16:12:06Z

Tests added, comments addressed, squashed and rebased.

eed3si9n · 2016-06-01T16:19:49Z

LGTM pending Travis

Could you add a release note to https://github.com/sbt/sbt/tree/0.13/notes/0.13.12/ under "Fixes with compatibility implications" header (see https://github.com/sbt/sbt/blob/0.13/notes/0.13.11.markdown for examples) please?

milessabin · 2016-06-01T16:22:42Z

Will do. Do you want that squashed into the same commit?

eed3si9n · 2016-06-01T16:23:35Z

Either is fine with me.

dwijnand · 2016-06-01T16:37:14Z

Could you also forward port this to https://github.com/sbt/librarymanagement please?

milessabin · 2016-06-01T19:52:55Z

I seem to have broken dependency-management/exclude-scala ... I don't really know what that test is testing ... help?

milessabin · 2016-06-02T09:02:48Z

sbt/src/sbt-test/dependency-management/exclude-scala/project/ExcludeScala.scala

+  lazy val root = Project("root", file(".")) settings(
+    libraryDependencies <++= baseDirectory(dependencies),
+    scalaVersion := "2.9.2",
+    ivyScala := { ivyScala.value map {_.copy(overrideScalaVersion = sbtPlugin.value)} },


@eed3si9n this fixes the test. Are you happy with the way that's being done?

Yea. That looks fine.

…Fixes sbt#2286.

dwijnand · 2016-06-02T14:37:58Z

no there're there

milessabin · 2016-06-03T08:28:23Z

Forward port can be found here and here.

Ref sbt#2634 updateSbtClassifiers uses an artificially created dependency graph set in classifiersModule. The problem is that ivyScala instance is reused from the outer scope that has the user project's scalaVersion as demonstrated as follows: scala> val is = (ivyScala in updateSbtClassifiers).eval is: Option[sbt.IvyScala] = Some(IvyScala(2.9.3,2.9.3,List(),true,false,true,org.scala-lang)) This change fixes sbt#2686 by redefining ivyScala with scalaVersion and scalaBinaryVersion scoped to updateSbtClassifiers task. The existing scripted test was modified to reproduce the bug.

Fixes sbt#2786. Ref sbt#2634. sbt 0.13.12 added Ivy mediator that enforces scalaOrganization and scalaVersion for Scala toolchain artifacts. This turns out to be a bit too aggressive because Ivy configurations can be used as an independent dependency graph that does not rely on the scalaVersion used by Compile configuration. By enforcing scalaVersion in those graph causes runtime failure. This change checks if the configuration extends Default, Compile, Provided, or Optional before enforcing scalaVersion.

Ref sbt#2634 updateSbtClassifiers uses an artificially created dependency graph set in classifiersModule. The problem is that ivyScala instance is reused from the outer scope that has the user project's scalaVersion as demonstrated as follows: scala> val is = (ivyScala in updateSbtClassifiers).eval is: Option[sbt.IvyScala] = Some(IvyScala(2.9.3,2.9.3,List(),true,false,true,org.scala-lang)) This change fixes sbt#2686 by redefining ivyScala with scalaVersion and scalaBinaryVersion scoped to updateSbtClassifiers task. The existing scripted test was modified to reproduce the bug.

There are a couple of settings / configs that affect this, summary below. The change in this PR seems to be the most narrow. `scalaModuleInfo.value.overrideScalaVersion` in sbt - affects how sbt to sets coursier's `forceScalaVersion` (see below) - used by librarymanagement.ivy. If true, add a OverrideScalaMediator See sbt#2634. Probably not relevant when using coursier. `autoScalaLibrary` setting in sbt - automatically add `scala-library` (or `scala3-library`) as a project dependency - also used for `forceScalaVersion` (see below) `CoursierConfiguration.autoScalaLibrary` - if `true` then Coursier `ResolutionParams.forceScalaVersion` is set to to `true` - initialized by sbt to `autoScalaLibrary.value && !ScalaArtifacts.isScala3(sv) && !Classpaths.isScala213(sv) && // added in this commit scalaModuleInfo.forall(_.overrideScalaVersion)` coursier `ResolutionParams.forceScalaVersion` - if true, `scala-library` / `scala-reflect` / `scala-compiler` / `scalap` are forced to the scala version, not actually resolved - for Scala 3, the `scala3-library` and `scala3-compiler` versions are forced

Fixes sbt#2786. Ref sbt#2634. sbt 0.13.12 added Ivy mediator that enforces scalaOrganization and scalaVersion for Scala toolchain artifacts. This turns out to be a bit too aggressive because Ivy configurations can be used as an independent dependency graph that does not rely on the scalaVersion used by Compile configuration. By enforcing scalaVersion in those graph causes runtime failure. This change checks if the configuration extends Default, Compile, Provided, or Optional before enforcing scalaVersion.

milessabin mentioned this pull request May 30, 2016

automatically provide dependencyOverrides for all scala-lang modules #2286

Closed

fommil reviewed May 30, 2016
View reviewed changes

eed3si9n added the ready label May 30, 2016

eed3si9n reviewed May 30, 2016
View reviewed changes

milessabin force-pushed the topic/scala-organization branch from 70f441f to ccd9f29 Compare May 31, 2016 09:26

milessabin reviewed May 31, 2016
View reviewed changes

milessabin force-pushed the topic/scala-organization branch from f886b8a to 0d52125 Compare June 1, 2016 16:11

milessabin reviewed Jun 2, 2016
View reviewed changes

milessabin force-pushed the topic/scala-organization branch from 533a0bb to f4929b0 Compare June 2, 2016 11:05

Override scala organization and version transitively at the Ivy level. …

e98b236

…Fixes sbt#2286.

milessabin force-pushed the topic/scala-organization branch from f4929b0 to e98b236 Compare June 2, 2016 11:07

milessabin mentioned this pull request Jun 3, 2016

Override scala organization and version transitively at the Ivy level #2635

Merged

milessabin deleted the topic/scala-organization branch June 3, 2016 08:28

dwijnand mentioned this pull request Jul 29, 2016

updateSbtClassifiers broken with sbt 0.13.12 on Scala 2.9 projects #2686

Closed

milessabin mentioned this pull request Aug 3, 2016

Fix for #2686 #2691

Closed

eed3si9n mentioned this pull request Aug 5, 2016

Fixes #2686 by resetting scalaVersion for updateSbtClassifiers #2692

Merged

dwijnand mentioned this pull request Oct 14, 2016

Cross-version issue with Scala 2.12.0-RC1 scalameta/scalafmt#485

Closed

olafurpg mentioned this pull request Oct 16, 2016

0.13.12 regression in resolving org.scala-lang:scala-library in custom configuration #2786

Closed

eed3si9n mentioned this pull request Nov 11, 2016

Apply scalaVersion enforcement to Compile related configs #2827

Merged

eed3si9n mentioned this pull request Nov 15, 2016

[sbt 1.0] Forward port 0.13 changes #2352

Closed

7 tasks

eed3si9n mentioned this pull request Jan 14, 2017

[fport] Fixes #2686 by resetting scalaVersion for updateSbtClassifiers #2915

Merged

dwijnand mentioned this pull request May 8, 2017

please publish for sbt 0.13 scalameta/scalafmt#923

Closed

eed3si9n mentioned this pull request Jan 3, 2018

Sbt problem with multiple scala versions #2869

Closed

liufengyun mentioned this pull request Dec 14, 2018

Weird versioning issues with SBT after upgrade to 2.12.8 scala/scala3#5612

Closed

This was referenced Jan 30, 2019

java.lang.NoClassDefFoundError: scala/math/Ordering$$anon$9 in runtime after adding sbt plugin #4525

Closed

adding the plugin changes the runtime behavior (withOverrideScalaVersion(false)) lucidsoftware/neo-sbt-scalafmt#78

Open

dwijnand mentioned this pull request Jul 2, 2020

Add support for the 3 last versions of scala for each binary version for rules scalacenter/scalafix#1174

Merged

eed3si9n mentioned this pull request Oct 20, 2021

Eviction error for downgrade of Scala 3.x #6694

Closed

eed3si9n mentioned this pull request Feb 3, 2023

SIP-51 - Drop Forwards Binary Compatibility of the Scala 2.13 Standard Library scala/improvement-proposals#54

Merged

mbovel mentioned this pull request Feb 26, 2026

Coursier backend doesn't substitute org.scala-lang artifacts when using a custom scalaOrganization #8821

Closed

Conversation

milessabin commented May 30, 2016

Uh oh!

lightbend-cla-validator commented May 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented May 30, 2016

Uh oh!

milessabin commented May 30, 2016

Uh oh!

ghost commented May 30, 2016

Uh oh!

milessabin commented May 30, 2016

Uh oh!

dwijnand commented May 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lightbend-cla-validator commented May 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lightbend-cla-validator commented May 31, 2016

Uh oh!

milessabin commented Jun 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eed3si9n commented Jun 1, 2016

Uh oh!

milessabin commented Jun 1, 2016

Uh oh!

eed3si9n commented Jun 1, 2016

Uh oh!

dwijnand commented Jun 1, 2016

Uh oh!

milessabin commented Jun 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwijnand commented Jun 2, 2016

Uh oh!

milessabin commented Jun 3, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

milessabin commented Jun 1, 2016 •

edited

Loading