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

Add strict aggregation from sbt-doge to SBT 1.0 #3698

Closed
ruippeixotog opened this Issue Oct 31, 2017 · 15 comments

Comments

Projects
None yet
3 participants
@ruippeixotog
Contributor

ruippeixotog commented Oct 31, 2017

I'm trying to migrate from sbt-doge to SBT 1.0, but I can't find a way to replace strict aggregation (+++). Using it seems to be the only way to build a project for a specific version (a thing usually done in Travis builds) in typical setups of a root project aggregating all sub-projects.

I think that the default behavior of ++ should be to do strict aggregation as I find it difficult to imagine an useful cross-project aggregation project otherwise. For compatibility purposes, one way to do this would be to add a strictAggregation sbt boolean setting that changed the behavior of the operator.

This issue follows from sbt/sbt-doge#18.

cc @eed3si9n @jroper @dwijnand

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Oct 31, 2017

If we are not already doing that, I think it makes sense for ++ 2.11.11 compile to do strict aggregation (aggregate across only the subprojects that supports Scala version 2.11.11).

To get the 0.13 behavior, people could use ++ 2.11.11! compile

At the same time, I've been caught off guard by the new ++2.11.11 not switching to the desired Scala version, so I think we might want to make the logging more verbose around ++.

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Oct 31, 2017

Sorry @eed3si9n, I should have provided an example to better explain the scenario to new people reading this. Consider the following build.sbt:

lazy val core = (project in file("core"))
  .settings(
    scalaVersion := "2.12.4",
    crossScalaVersions := Seq("2.11.11", "2.12.4"))

// some module with a 2.12-only library dependency
lazy val module = (project in file("module"))
  .dependsOn(core)
  .settings(
    scalaVersion := "2.12.4",
    crossScalaVersions := Seq("2.12.4"))

The output for ++2.11.11 compile in this scenario is:

(sbt) sbt-test ❯ ++2.11.11 compile
[info] Setting Scala version to 2.11.11 on 1 projects.
[info] Excluded 2 projects, run ++ 2.11.11 -v for more details.
[info] Reapplying settings...
[info] Set current project to sbt-test (in build file:/Users/rui/Downloads/sbt-test/)
[info] Updating module
[info] Resolved module dependencies
[error] coursier.ResolutionException: Encountered 1 error(s) in dependency resolution:
[error]     core:core_2.12:0.1-SNAPSHOT:
[error]         not found:
[error]             /Users/rui/.ivy2/local/core/core_2.12/0.1-SNAPSHOT/ivys/ivy.xml
[error]             https://repo1.maven.org/maven2/core/core_2.12/0.1-SNAPSHOT/core_2.12-0.1-SNAPSHOT.pom
[error] (module/*:coursierResolutions) coursier.ResolutionException: Encountered 1 error(s) in dependency resolution:
[error]     core:core_2.12:0.1-SNAPSHOT:
[error]         not found:
[error]             /Users/rui/.ivy2/local/core/core_2.12/0.1-SNAPSHOT/ivys/ivy.xml
[error]             https://repo1.maven.org/maven2/core/core_2.12/0.1-SNAPSHOT/core_2.12-0.1-SNAPSHOT.pom

++ 2.11.11! compile isn't adequate in this scenario because it would force module to use Scala 2.11.11 when it has an incompatible dependency. sbt-doge solves this issue with its strict aggregation operator (+++), which removes from aggregation projects the subprojects without a compatible version in crossScalaVersions.

The real-life use case for this is to write things like this in Travis builds. Travis provides a way to list Scala versions to be tested and I would like to keep using it instead of merging them all and doing +test in a single build.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Oct 31, 2017

I think we are on the same page. I wrote sbt-doge because I thought +compile should first iterate over the Scala versions, and then aggregate only the subprojects that support a particular Scala version.

Similar argument can be made for ++2.11.11 compile, and in your example skip the aggregation of "module" subproject. To put it in terms of steps, problem, expectation.

steps

lazy val core = (project in file("core"))
  .settings(
    scalaVersion := "2.12.4",
    crossScalaVersions := Seq("2.11.11", "2.12.4"))

// some module with a 2.12-only library dependency
lazy val module = (project in file("module"))
  .dependsOn(core)
  .settings(
    scalaVersion := "2.12.4",
    crossScalaVersions := Seq("2.12.4"))
  1. Run ++ 2.11.11 compile.

problem

The build fails:

(sbt) sbt-test ❯ ++2.11.11 compile
[info] Setting Scala version to 2.11.11 on 1 projects.
[info] Excluded 2 projects, run ++ 2.11.11 -v for more details.
[info] Reapplying settings...
[info] Set current project to sbt-test (in build file:/Users/rui/Downloads/sbt-test/)
[info] Updating module
[info] Resolved module dependencies
[error] coursier.ResolutionException: Encountered 1 error(s) in dependency resolution:
[error]     core:core_2.12:0.1-SNAPSHOT:
[error]         not found:
[error]             /Users/rui/.ivy2/local/core/core_2.12/0.1-SNAPSHOT/ivys/ivy.xml
[error]             https://repo1.maven.org/maven2/core/core_2.12/0.1-SNAPSHOT/core_2.12-0.1-SNAPSHOT.pom
[error] (module/*:coursierResolutions) coursier.ResolutionException: Encountered 1 error(s) in dependency resolution:
[error]     core:core_2.12:0.1-SNAPSHOT:
[error]         not found:
[error]             /Users/rui/.ivy2/local/core/core_2.12/0.1-SNAPSHOT/ivys/ivy.xml
[error]             https://repo1.maven.org/maven2/core/core_2.12/0.1-SNAPSHOT/core_2.12-0.1-SNAPSHOT.pom

expectation

Whenever ++ <scala-version> <command> is invoked with command,

  1. Remember the current Scala versions across all subprojects.
  2. Set Scala version of the subprojects to the given <scala-version> only if it's listed in crossScalaVersions.
  3. Run <command> only on the subprojects that supports <scala-version>.
  4. Reset all subprojects' Scala versions back to original.

TBonnin added a commit to guardian/facia-scala-client that referenced this issue Nov 1, 2017

Upgrade to sbt 1.0
Also explicitely set scalaVersion and crossScalaVersions on every project
An issue in sbt 0.13.5 prevented us to crosscompile some libs aggregated
in a root project (See: sbt/sbt#1448)
This issue has been fixed in sbt 1.0 (See: sbt/sbt#3698)
@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 4, 2018

Hi! It's been 4 months since the last update on this. Is there any plan to add this feature to SBT in a near future? The pureconfig build is still stuck on SBT 0.13 because of this.

I'm willing to try to implement this feature and submit a PR if this is not a priority for the SBT team, but I'd need some guidance on where and how this could be implemented.

@eed3si9n eed3si9n added the help wanted label Mar 4, 2018

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 4, 2018

I'm willing to try to implement this feature and submit a PR if this is not a priority for the SBT team, but I'd need some guidance on where and how this could be implemented.

It's not a priority for sbt core devs. We'd be happy to assist you in this.
A good starting point is #2613 where doge gets merged into sbt 1. You can see which code does what and how it should be tested.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 4, 2018

If you have any other questions feel free to pop into https://gitter.im/sbt/sbt-contrib

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 4, 2018

Thanks @eed3si9n! I'll give it a try when I find the time for it.

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 6, 2018

@eed3si9n just to confirm one thing: in your specification above, you mentioned "4. Reset all subprojects' Scala versions back to original". However, ++ currently doesn't work this way - it sets the Scala version to the one specified (even on projects without it listed on their crossScalaVersions) and leaves that version set after execution. Is it ok to change the behavior of ++ for future versions?

sbt-doge leaves this operator working as it was in SBT 0.13 and adds a new +++ command that does what you described. I'd rather see ++ working as you describe, I'm just worried about backward compatibility.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 6, 2018

I guess we can interpret ++ <scala-version> as a special case of ++ <scala-version> <command> where <command> is shell. Not that I am suggesting we actually call shell command, but theoretically would that make sense? Similar to calling sbt without commands.

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 6, 2018

That seems sound, semantically speaking. In SBT shell environments the difference between the two is clear, while both forms can still be used from a Unix shell by calling either sbt "++ <scala-version>" <command> or sbt "++ <scala-version> <command>". If you think it's ok to break compatibility for scripts expecting ++ to always persist a Scala version, I think I can implement this.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 6, 2018

If you think it's ok to break compatibility for scripts expecting ++ to always persist a Scala version

This is a new behavior for ++, so it will not break runtime behavior of existing builds right?
Am I missing something?

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 6, 2018

I think it does. In the previous example, with SBT 1.1.1:

(sbt) core ❯ show scalaVersion
[info] 2.12.4
(sbt) core ❯ ++ 2.11.11 compile
(...)
[success] Total time: 1 s, completed Mar 6, 2018 9:03:44 PM
(sbt) core ❯ show scalaVersion
[info] 2.11.11

If I understood correctly, after the change we're considering the last command should print "2.12.4". This should not be a common scenario in real scripts (as people typically write something like sbt ++$SCALA_VERSION <task1> <task2...>, but it can break some more contrived examples.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 6, 2018

ok. It's been a while so I had forgotten that ++ takes <command> already.
I guess we should try to keep the compatibility, and change the scalaVersion afterwards then.

@ruippeixotog

This comment has been minimized.

Contributor

ruippeixotog commented Mar 6, 2018

Ok! So, currently ++ <scala-version> <command> is equivalent to ; ++ <scala-version>; <command>, which is handy when running SBT commands directly through the command-line.

To implement strict aggregation in ++, I suppose I'll have to either:

  • modify the State to remove non-compatible projects from aggregations, making this "permanent" in the SBT shell unless I explicitly revert the changes afterwards;
  • expand <command> to something like all <project1>/<command> <project2>/<command> ... without touching the State.

What's your opinion on this? IMO, the second option seems more reasonable if the intention is to start distinguishing the two ways of writing the expression above, even though it's strange to have scalaVersion changing "permanently" but not the aggregation.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Mar 6, 2018

Definitely option 2. sbt is meant to work from the sbt shell, so removing subprojects would be bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment