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

[sbt 1.0] Replaced cross building support with sbt-doge #2613

Merged
merged 1 commit into from May 17, 2016

Conversation

Projects
None yet
4 participants
@jroper
Copy link
Member

commented May 13, 2016

The command names are kept as +, ++, but the actual cross building approach, of only building projects that report being binary compatible to the scala version, is taken from sbt-doge.

We've been using sbt-doge for over a year on Play, and it works well for us.

@jroper jroper force-pushed the jroper:sbt-doge branch from bf5225a to 23e72aa May 13, 2016

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 13, 2016

+1 but it's been fun trolling sbt with so prefix.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 13, 2016

The failure on Travis scripted compiler-project/separate-analysis-per-scala seems legit. I can reproduce it on my machine too.

@jroper jroper force-pushed the jroper:sbt-doge branch from 23e72aa to 138f1a8 May 13, 2016

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 13, 2016

Yep, it was legit. I've fixed and pushed, but I think it's introduced another issue.

Looking over the code carefully, sbt-doge could do things a lot better. The question is, whether we want to maintain the exact same old behaviour or not.

Previously, you could say ++ 2.12.0-M4, and even projects that weren't configured to cross build against 2.12 would cross build against 2.12. If we're prepared to forfeit this behaviour, I think we can do a much better implementation of cross building. To me, it seems reasonable, you should only be able to cross build against versions that are binary compatible with versions that the build is configured to cross build against. You can always reconfigure the cross versions if you want to introduce a new one. I don't think there should be any reason to, in every day development, cross build against versions that aren't configured.

If we are prepared to relax that behaviour, then I propose that we modify the switch command to simply set the version for all projects that are compatible with that scala version, not all projects.

@dwijnand

This comment has been minimized.

Copy link
Member

commented May 13, 2016

👍, including limiting ++ to binary compatible versions, sounds reasonable, that way play's sbt-plugin (which by-the-by would be nice if it were named sbt-play) doesn't try to compile with 2.12.0-M4.

The only fall out is 2.12.0-M4's binary version is 2.12.0-M4, so there's no way to preconfigure crossScalaVersions when 2.12.0-M5 is out and you want to ad-hoc try it out in the SBT shell.. you'd have to also modify crossScalaVersions.

Also perhaps let's make sure this doesn't affect dbuild/community-build with their adhoc Scala version testing.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 13, 2016

Previously, you could say ++ 2.12.0-M4, and even projects that weren't configured to cross build against 2.12 would cross build against 2.12. If we're prepared to forfeit this behaviour, I think we can do a much better implementation of cross building.

Let's review the various ++ behaviors and use cases:

  • sbt 0.13's ++ 2.12.0-M4 command: Switches all subprojects' scalaVersion
  • sbt-doge's ++ 2.12.0-M4 command (aka wow): Switches only the current project's aggregated subprojects
  • sbt-doge's +++ 2.12.0-M4 command (aka plz): Switches only the current project's aggregated subprojects that contains 2.12.0-M4 in the crossScalaVersions list

There are several use cases for the ++-family of commands.

  1. switch: You have a complex graph of subprojects with various crossScalaVersions, some supporting only 2.10, only 2.11, both, etc; and you want to switch from one Scala version mode to another.
  2. try: You have a relatively simple graph of subprojects; and you want to try another version of Scala without changing the build definition.

I would say the use cases are 50:50 in terms of switch vs try, and @jroper's +++ implementation is geared more towards switching scenario, but it loses the trying ability. Here's the new "relaxed" ++ one could look like:

sbtRoot> ++2.12.0-M4
[info] Setting version to 2.12.0-M4:
[info]     actionsProj (2.11.8, 2.10.6)
[info]   * sbtRoot (2.11.8, 2.10.6)
[info] Excluded from switching:
[info]     scriptedPluginProj (2.10.6)
[info]     testAgentProj (2.10.6)
[info] Use '++2.12.0-M4!' to switch excluded projects too.
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/) 
sbtRoot> scalaVersion
[info] actionsProj/*:scalaVersion
[info]  2.12.0-M4
[info] sbtRoot/*:scalaVersion
[info]  2.12.0-M4
[info] scriptedPluginProj/*:scalaVersion
[info]  2.10.6
[info] testAgentProj/*:scalaVersion
[info]  2.10.6
sbtRoot> ++2.12.0-M4!
[info] Setting version to 2.12.0-M4:
[info]     actionsProj (2.11.8, 2.10.6)
[info]   * sbtRoot (2.11.8, 2.10.6)
[info]     scriptedPluginProj (2.10.6)
[info]     testAgentProj (2.10.6)
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/) 

Suppose sbtRoot and actionsProj are on 2.11.8 first. The new ++ will look at the aggregate list, and filter it down to a subset that has binary compatible Scala version and has the same as the current project. In this example, let's say scriptedPluginProj and testAgentProj are excluded because they're on 2.10 (I am guessing this is what James meant by "all projects that are compatible with that scala version"). Importantly, it will print out what projects were switched.

To get similar behavior as the old 0.13, we can also provide "!" variant that works on the aggregated list of the current projects.

@dwijnand

This comment has been minimized.

Copy link
Member

commented May 13, 2016

👍 for !

On Fri, 13 May 2016, 12:33 eugene yokota, notifications@github.com wrote:

Previously, you could say ++ 2.12.0-M4, and even projects that weren't
configured to cross build against 2.12 would cross build against 2.12. If
we're prepared to forfeit this behaviour, I think we can do a much better
implementation of cross building.

Let's review the various ++ behaviors and use cases:

  • sbt 0.13's ++ 2.12.0-M4 command: Switches all subprojects'
    scalaVersion
  • sbt-doge's ++ 2.12.0-M4 command (aka wow): Switches only the current
    project's aggregated subprojects
  • sbt-doge's +++ 2.12.0-M4 command (aka plz): Switches only the
    current project's aggregated subprojects that contains 2.12.0-M4 in
    the crossScalaVersions list

There are several use cases for the ++-family of command.

  1. switch: You have a complex graph of subprojects with various
    crossScalaVersions, some supporting only 2.10, only 2.11, both, etc;
    and you want to switch from one Scala version mode to another.
  2. try: You have a relatively simple graph of subprojects; and you
    want to try another version of Scala without changing the build
    definition.

I would say the use cases are 50:50 in terms of switch vs try, and @jroper
https://github.com/jroper's +++ implementation is geared more towards
switching scenario, but it loses the trying ability. Here's the new
"relaxed" ++ one could look like:

sbtRoot> ++2.12.0-M4
[info] Setting version to 2.12.0-M4:
[info] actionsProj (2.11.8, 2.10.6)
[info] * sbtRoot (2.11.8, 2.10.6)
[info] Excluded from switching:
[info] scriptedPluginProj (2.10.6)
[info] testAgentProj (2.10.6)
[info] Use '++2.12.0-M4!' to switch excluded projects too.
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/)
sbtRoot> scalaVersion
[info] actionsProj/:scalaVersion
[info] 2.12.0-M4
[info] sbtRoot/
:scalaVersion
[info] 2.12.0-M4
[info] scriptedPluginProj/:scalaVersion
[info] 2.10.6
[info] testAgentProj/
:scalaVersion
[info] 2.10.6
sbtRoot> ++2.12.0-M4!
[info] Setting version to 2.12.0-M4:
[info] actionsProj (2.11.8, 2.10.6)
[info] * sbtRoot (2.11.8, 2.10.6)
[info] scriptedPluginProj (2.10.6)
[info] testAgentProj (2.10.6)
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/)

Suppose sbtRoot and actionsProj are on 2.11.8 first. The new ++ will look
at the aggregate list, and filter it down to a subset that has binary
compatible Scala version and has the same as the current project. In this
example, let's say scriptedPluginProj and testAgentProj are excluded
because they're on 2.10 (I am guessing this is what James meant by "all
projects that are compatible with that scala version"). Importantly, it
will print out what projects were switched.

To get similar behavior as the old 0.13, we can also provide "!" variant
that works on the aggregated list of the current projects.


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#2613 (comment)

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

I think the logging there may be a little too much - it's fine for a project with 4 sub projects, but consider Play and Akka, which have 20+ sub projects, it's a lot of noise that adds no value 99% of the time.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 16, 2016

Since we are actively defying the user's command to switch Scala version, I think we need to provide as much explanation as possible. We can remove some new lines and format it multi column based on terminal width maybe. sbt users are used to combing through hundreds of lines of compiler errors, I don't think they'd flinch with 20 lines of subproject list.

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

Could we log it as debug then?

sbt users are used to combing through hundreds of lines of compiler errors

I think a better way of saying this is "sbt users use sbt in spite of always having to comb through hundreds of lines of compiler errors". I've never met an sbt user who thinks the amount that sbt logs is a good thing, in fact many projects (Play included) go out of their way to try and reduce the amount of noise that sbt makes (eg, so that Travis doesn't fail because the build outputs more than 10K lines). I don't think we should do anything to contribute to making it worse.

@jroper jroper force-pushed the jroper:sbt-doge branch from 138f1a8 to db43c69 May 16, 2016

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

I've pushed an updated PR (it doesn't log which projects are using the new Scala version yet).

This PR actually greatly changes the way the version overriding is done. Previously, it set scalaVersion in Global, and then modified each project to delegate to that. Now, scalaVersion in Global is never touched, rather, each project that is having its Scala version updated will have scalaVersion in <project> set. Additionally, appendRaw is filtered each time this is done, so the overridden Scala version settings don't build up over time.

The ! operator, appended to the end of the Scala version of Scala home, will cause all projects regardless of compatibility to have their Scala version updated. This includes ThisBuild.

One thing I'm not sure about, I'm updating the versions for all projects in all builds. Not sure if that's the right thing to do, or if just projects in the current build are updated.

Another thing, the switch command will update everything by default, but if a command is passed to it, it only updates aggregates of the current project, or if that command is a command on a specific project, then it just updates that project. Not sure if there are any problems with that behaviour or not.

Also, the way the state is reset after + is run, the current appendRaw settings are captured and put into the session, then a new +- command has been defined that restores the session to those settings. So + should now do a much better job of leaving the build in the state it was before it was run.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 16, 2016

Travis Log is a whole another story, because it ends up seeing a lot of the dependency logs that normally you wouldn't see because of caching or displayed in place on CLI (Resolving ..).
I still think we should display something because it is a departure from 0.13 behavior. Perhaps a middle ground would be displaying a summary message like this?:

sbtRoot> ++2.12.0-M4
[info] Switched 2 and excluded 2 subprojects.
[info] Run '++2.12.0-M4 -v' for more details.
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/) 
sbtRoot> ++2.12.0-M4 -v
[info] Switched 2 and excluded 2 subprojects.
[info] Setting version to 2.12.0-M4:
[info]     actionsProj (2.11.8, 2.10.6)
[info]   * sbtRoot (2.11.8, 2.10.6)
[info] Excluded from switching:
[info]     scriptedPluginProj (2.10.6)
[info]     testAgentProj (2.10.6)
[info] Use '++2.12.0-M4!' to switch excluded projects too.
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/) 
sbtRoot> debug
sbtRoot> ++2.12.0-M4
[info] Switched 2 and excluded 2 subprojects.
[info] Run '++2.12.0-M4 -v' for more details.
[debug] Setting version to 2.12.0-M4:
[debug]     actionsProj (2.11.8, 2.10.6)
[debug]   * sbtRoot (2.11.8, 2.10.6)
[debug] Excluded from switching:
[debug]     scriptedPluginProj (2.10.6)
[debug]     testAgentProj (2.10.6)
[debug] Use '++2.12.0-M4!' to switch excluded projects too.
[info] Reapplying settings...
[info] Set current project to sbtRoot (in build file:/Users/xxx/work/sbt-modules/sbt/) 

@jroper jroper force-pushed the jroper:sbt-doge branch from db43c69 to 35a8c15 May 16, 2016

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 16, 2016

One thing I'm not sure about, I'm updating the versions for all projects in all builds. Not sure if that's the right thing to do, or if just projects in the current build are updated.

One of the feature of sbt-doge was that it limited the scope of switching to just the aggregated subproject of the current project. I thought it would be most natural, because that's how something like clean would propagate. Then again, if someone doesn't have all of the transitive subproject dependencies listed in aggregate(...) you could end up with half-switched build.

For all-vs-current in your case, are you concerned about ProjectRef that goes out to Git and stuff?

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

For all-vs-current in your case, are you concerned about ProjectRef that goes out to Git and stuff?

I don't know actually - I don't have a good grasp of what all the uses for project refs are. The primary use case that I have for them is in projects that have both sbt plugins and normal libraries, where you want to use the sbt plugin in your build. For example, the Play docs project, when it compiles the Play code samples, needs to use the Play sbt plugin to compile routes and things like that. So the docs project is defined as a separate build because it can't be part of the main build because the sbt plugin it wants to use is part of the main project. In this case, rather than having to do publishLocal each time you make a change before you can check whether the documentation still compiles, we define project refs from the documentation build, both from the plugins.sbt to the Play sbt plugin, and from the main build.sbt to the main Play runtime libraries. In this case, I think we would need the Scala version to be applied for all builds, not just the current build, if we ever wanted to cross build our documentation code samples, which we do.

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

@adriaanm This PR may (or may not) impact your team with the community builds and/or day to day testing.

As it stands, the behaviour of ++ is going to change to only switch the Scala version for projects that are binary compatible with the selected Scala version, determined using crossScalaVersions. So testing with a new arbitrary Scala version won't work. The behaviour can be changed by appending a ! to the Scala version, which will force all projects to switch to that version regardless of whether they are reported as being compatible or not.

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

Something also that may break with this change is when you have:

scalaVersion := "2.11.8"
crossScalaVersions = Seq(scalaVersion.value, "2.10.5")

When you run ++ 2.10.5, crossScalaVersions will become Seq("2.10.5", "2.10.5"). When you then run ++ 2.11.8, the project won't get the Scala version updated because there's no version in crossScalaVersions that's binary compatible with 2.11.8.

This problem actually already existed in some way, since if you ran ++ 2.10.5 followed by + compile, it would run the build against 2.10.5 twice. The impact is probably more noticable now since the problem could be exhibited in a clean session, when simply running + compile, whereas previously it wouldn't impact a clean session. But either way, crossScalaVersions should not depend on scalaVersion, and we should probably put a note in the docs to this effect. So I'm not sure that this is something we need to worry about.

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

Ok, I've got this:

> ++2.11.8
[info] Setting Scala version to 2.11.8 on 2 projects.
[info] Excluded 1 projects, run ++ 2.11.8 -v for more details.
[info] Reapplying settings...
[info] Set current project to rootProj (in build file:/tmp/sbt-backup/)
> ++2.11.8 -v
[info] Setting Scala version to 2.11.8 on 2 projects.
[info] Switching Scala version on:
[info]   * rootProj (2.11.8)
[info]     libProj (2.11.8, 2.10.4)
[info] Excluding projects:
[info]     fooPlugin (2.10.4)
[info] Reapplying settings...
[info] Set current project to rootProj (in build file:/tmp/sbt-backup/)
> ++2.11.8!
[info] Forcing Scala version to 2.11.8 on all projects.
[info] Reapplying settings...
[info] Set current project to rootProj (in build file:/tmp/sbt-backup/)
> debug
[debug] > shell
> ++2.11.8
[debug] > ++2.11.8
[info] Setting Scala version to 2.11.8 on 2 projects.
[info] Excluded 1 projects, run ++ 2.11.8 -v for more details.
[debug] Switching Scala version on:
[debug]   * rootProj (2.11.8)
[debug]     libProj (2.11.8, 2.10.4)
[debug] Excluding projects:
[debug]     fooPlugin (2.10.4)
[info] Reapplying settings...
[info] Set current project to rootProj (in build file:/tmp/sbt-backup/)
[debug] > shell
> 
@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 16, 2016

Nice!

@jroper jroper force-pushed the jroper:sbt-doge branch 2 times, most recently from 00d797d to 5b5b465 May 16, 2016

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

Updated this PR with the logging changes. Also, I modified the behaviour again, in no circumstances does it update the Scala version for just the current project or aggregate projects, since if there are any dependencies that aren't aggregated, this will never be the right thing to do. If the Scala version is going to change anywhere, it should change everywhere that that change is allowed.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 16, 2016

Does this change affect the behavior on ;clean;+compile;+publish compared to sbt-doge?

@adriaanm

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

@adriaanm This PR may (or may not) impact your team with the community builds and/or day to day testing.

Thanks for the heads up. I believe it would only affect us to the extent that it affects dbuild. I don't understand dbuild's inner working enough to know, but I assume it doesn't :-)

@adriaanm

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

That said, this looks like a great sbt feature for all cross-versioned projects! Excited for what it could mean in terms of uptake of new Scala versions.

@jroper jroper force-pushed the jroper:sbt-doge branch from 5b5b465 to e21c871 May 16, 2016

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 16, 2016

@eed3si9n No, you should notice no change - the only thing you may notice is that it's more hygienic, it leaves state in a better state after running it.

@jroper

This comment has been minimized.

Copy link
Member Author

commented May 17, 2016

I don't see any outstanding objections, and I've rebased this and it's passing CI, ready to merge?

btw, I'm sure some problems will be uncovered with this as people start upgrading and using it, that's why I moved to get it into sbt 1.0.x as quickly as I could, so we can uncover those problems before 1.0.0 is released.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 17, 2016

LGTM

@eed3si9n eed3si9n merged commit 5ecbf50 into sbt:1.0.x May 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n removed the in progress label May 17, 2016

@jroper jroper deleted the jroper:sbt-doge branch May 17, 2016

@eed3si9n eed3si9n changed the title Replaced cross building support with sbt-doge [sbt 1.0] Replaced cross building support with sbt-doge May 17, 2016

@xuwei-k xuwei-k referenced this pull request Jul 3, 2017

Closed

cross build for sbt 1.0 #51

@xuwei-k xuwei-k referenced this pull request Jul 25, 2017

Closed

Cross build for sbt 1.0 #207

xuwei-k added a commit to xuwei-k/website that referenced this pull request Nov 3, 2017

fix Task-Graph.md example
```
> ++2.11.8
[info] Setting Scala version to 2.11.8 on 0 projects.
[info] Excluded 1 projects, run ++ 2.11.8 -v for more details.
[info] Reapplying settings...
```

> The behaviour of ++ is changed so that it only updates the Scala version of projects that support that Scala version, but the Scala version can be post fixed with ! to force it to change for all projects.

- http://www.scala-sbt.org/1.x/docs/sbt-1.0-Release-Notes.html
- sbt/sbt#2613

@xuwei-k xuwei-k referenced this pull request Nov 3, 2017

Merged

fix Task-Graph.md example #546

xuwei-k added a commit to sbt/website that referenced this pull request Nov 3, 2017

fix Task-Graph.md example (#546)
```
> ++2.11.8
[info] Setting Scala version to 2.11.8 on 0 projects.
[info] Excluded 1 projects, run ++ 2.11.8 -v for more details.
[info] Reapplying settings...
```

> The behaviour of ++ is changed so that it only updates the Scala version of projects that support that Scala version, but the Scala version can be post fixed with ! to force it to change for all projects.

- http://www.scala-sbt.org/1.x/docs/sbt-1.0-Release-Notes.html
- sbt/sbt#2613

@milessabin milessabin referenced this pull request Feb 12, 2018

Merged

sbt 1.1.1 #813

@xuwei-k xuwei-k referenced this pull request Mar 4, 2018

Merged

sbt 1.1.1 #155

@tanishiking tanishiking referenced this pull request Aug 1, 2018

Merged

Upgrade sbt to 1.2.0 #1247

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.