Skip to content

Conversation

milessabin
Copy link
Contributor

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

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

@@ -7,7 +7,7 @@ import Sxr.sxr
// but can be shared across the multi projects.
def buildLevelSettings: Seq[Setting[_]] = inThisBuild(Seq(
organization := "org.scala-sbt",
version := "0.13.12-SNAPSHOT",
version := "0.13.12-devel",
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably didn't mean to commit this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@ghost
Copy link

ghost commented May 30, 2016

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
Copy link
Contributor Author

@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
Copy link

ghost commented May 30, 2016

@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
Copy link
Contributor Author

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

@eed3si9n eed3si9n added the ready label May 30, 2016
@dwijnand
Copy link
Member

Nice and simple, nice.

if (check.filterImplicit)
excludeScalaJars(module, check.configurations)
if (check.overrideScalaVersion)
overrideScalaVersion(module, check.scalaFullVersion)
if (true || check.overrideScalaVersion)
Copy link
Member

Choose a reason for hiding this comment

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

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.

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

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

@@ -1193,6 +1190,9 @@ object Classpaths {
projectDescriptors <<= depMap,
updateConfiguration := new UpdateConfiguration(retrieveConfiguration.value, false, ivyLoggingLevel.value),
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))
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@lightbend-cla-validator

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 milessabin force-pushed the topic/scala-organization branch from f886b8a to 0d52125 Compare June 1, 2016 16:11
@milessabin
Copy link
Contributor Author

milessabin commented Jun 1, 2016

Tests added, comments addressed, squashed and rebased.

@eed3si9n
Copy link
Member

eed3si9n commented Jun 1, 2016

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
Copy link
Contributor Author

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

@eed3si9n
Copy link
Member

eed3si9n commented Jun 1, 2016

Either is fine with me.

@dwijnand
Copy link
Member

dwijnand commented Jun 1, 2016

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

@milessabin
Copy link
Contributor Author

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

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

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Yea. That looks fine.

@milessabin milessabin force-pushed the topic/scala-organization branch from 533a0bb to f4929b0 Compare June 2, 2016 11:05
@milessabin milessabin force-pushed the topic/scala-organization branch from f4929b0 to e98b236 Compare June 2, 2016 11:07
@eed3si9n
Copy link
Member

eed3si9n commented Jun 2, 2016

did we miss the release notes?

@dwijnand
Copy link
Member

dwijnand commented Jun 2, 2016

no there're there

@milessabin
Copy link
Contributor Author

Forward port can be found here and here.

@milessabin milessabin deleted the topic/scala-organization branch June 3, 2016 08:28
@milessabin milessabin mentioned this pull request Aug 3, 2016
eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Aug 5, 2016
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.
eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Nov 11, 2016
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.
eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Nov 11, 2016
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.
eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Jan 14, 2017
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.
lrytz added a commit to lrytz/sbt that referenced this pull request Jan 22, 2024
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
lrytz added a commit to lrytz/sbt that referenced this pull request Jan 23, 2024
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
lrytz added a commit to lrytz/sbt that referenced this pull request Jan 23, 2024
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
lrytz added a commit to lrytz/sbt that referenced this pull request Feb 26, 2024
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
adpi2 pushed a commit to adpi2/sbt that referenced this pull request Oct 9, 2024
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.
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 this pull request may close these issues.

5 participants