-
Notifications
You must be signed in to change notification settings - Fork 442
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
Upgrade sbt, scala and sbt-ci-release to avoid deprecated repo.scala-sbt.org #1566
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
8ec8199
Update gitignore
mkurz b2107fb
Upgrade sbt, scala and sbt-ci-release
mkurz fc6f13f
Upgrade sbt-ghpages, sbt-site(-sphinx)
mkurz 45daeb6
Keep outdated sbt versions in test projects
mkurz e7eab95
Process args escaping changed starting in sbt 1.6.0-M1
mkurz 7f17f57
Merge branch 'master' into upgrade-sbt
muuki88 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,3 +13,4 @@ target/ | |
.ensime* | ||
.bloop/* | ||
.metals/* | ||
.bsp/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
integration-tests-ansible/test-project-play-rpm/project/build.properties
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
sbt.version=0.13.8 | ||
sbt.version=1.9.9 |
2 changes: 0 additions & 2 deletions
2
integration-tests-ansible/test-project-play-rpm/project/plugins.sbt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
sbt.version=1.5.4 | ||
sbt.version=1.9.9 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I found what made the test fail on AppVeyor running Windows.
For the record, the error message was:
Turns out, this has nothing to do with this pull request per se, you can easily reproduce this error even with sbt-native-packager 1.9.16 on the current main branch, without this PR applied:
main
branchI did check and debug: The process args and environment passed here are 100% the same for both sbt 1.5.8 and 1.6.0-M1, nothing changed... So why does sbt 1.6+ fail?
Before we go deeper, the technical reason why it fails is because
<
and>
are handled like normal pipes starting with sbt 1.6+ and not passed as string.So why is that?
Let's look at the Windows specific code that runs a process, this line. You can see there is a check for a
SecurityManager
, and based on its existence things behave differently.So if a
SecurityManager
is available, the process will be started differently: the process args will be handled and escaped differently.Now.. it turns out until sbt 1.5.8, sbt did install a
SecurityManager
, but starting with sbt 1.6.0-M1 it does not anymore.Look at this commit in sbt, you will see in
Main.scala
theTrapExit.installManager()
got removed.So this explains why the test started to fail with sbt 1.6.+
Now the thing is, assuming most people upgraded to sbt 1.6 or newer already anyway, this test now just got adjusted to the current real world sbt behaviour.
The reason why the scripted test fails with this pull request is because I upgraded sbt to 1.9.9 and by default the scripted tests are run with the sbt version of the main project. Like showed above however, this behaviour already surfaces in any project that uses sbt-native-packager with sbt 1.6+.
(I mean in theory I could make the scripted test here keep running with sbt 1.5.4, so I wouldn't have to adjust the test itself, but that does not make any sense IMHO).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, TIL. I agree it doesn't make sense to depend on deprecated
SecurityManager
behavior.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, TIL too, took me hours to figure out what the hell is going on...