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

Upgrade sbt, scala and sbt-ci-release to avoid deprecated repo.scala-sbt.org #1566

Merged
merged 6 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ target/
.ensime*
.bloop/*
.metals/*
.bsp/
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ organization := "com.github.sbt"
homepage := Some(url("https://github.com/sbt/sbt-native-packager"))

Global / onChangedBuildSource := ReloadOnSourceChanges
Global / scalaVersion := "2.12.12"
Global / scalaVersion := "2.12.19"

// crossBuildingSettings
crossSbtVersions := Vector("1.1.6")
Expand Down
2 changes: 0 additions & 2 deletions integration-tests-ansible/test-project-play-rpm/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ scalacOptions in ThisBuild ++= Seq(
"-Xlint"
)

resolvers in ThisBuild += Resolver.typesafeRepo("releases")

name := "test-project-play-rpm"

description := "Demo of RPM packaging"
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=0.13.8
sbt.version=1.9.9
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
resolvers += Resolver.typesafeRepo("releases")

libraryDependencies <+= Def.setting[ModuleID] {
Defaults
.sbtPluginExtra(
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=1.5.4
sbt.version=1.9.9
6 changes: 3 additions & 3 deletions project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
addSbtPlugin("com.typesafe.sbt" % "sbt-ghpages" % "0.6.3")
addSbtPlugin("com.typesafe.sbt" % "sbt-site" % "1.4.0")
addSbtPlugin("com.github.sbt" % "sbt-ghpages" % "0.8.0")
addSbtPlugin("com.github.sbt" % "sbt-site-sphinx" % "1.7.0")

// releasing
addSbtPlugin("com.geirsson" % "sbt-ci-release" % "1.5.7")
addSbtPlugin("com.github.sbt" % "sbt-ci-release" % "1.5.12")

libraryDependencies += "org.scala-sbt" %% "scripted-plugin" % sbtVersion.value

Expand Down
6 changes: 2 additions & 4 deletions src/sbt-test/windows/test-bat-template/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,16 @@ TaskKey[Unit]("checkScript") := {
checkOutput("include symbols on -D", Seq("-Dtest.hoge=\\[]!< >%"), "property(test.hoge) is [\\[]!< >%]\nSUCCESS!")
checkOutput("include symbols on normal args", Seq("\"\\[]!< >%\""), "arg #0 is [\\[]!< >%]\nSUCCESS!")

/* fails test because symbols '<' and '>' cannot be properly escaped during cmd execution
checkOutput(
"include symbols with double quote",
Seq("-Dtest.huga=\"[]!<>%\""),
"property(test.huga) is [[]!<>%]\nSUCCESS!"
)
*/

checkOutput(
"include symbols with double quote2",
Seq("-Dtest.hoge=\\[]!< >%", "\"\\[]!< >%\"", "-Dtest.huga=\\[]!<>%"),
"arg #0 is [\\[]!< >%]\nproperty(test.hoge) is [\\[]!< >%]\nproperty(test.huga) is [\\[]!<>%]\nSUCCESS!"
Seq("-Dtest.hoge=\\[]!< >%", "\"\\[]!< >%\"", "-Dtest.huga=\\[]!%"),
"arg #0 is [\\[]!< >%]\nproperty(test.hoge) is [\\[]!< >%]\nproperty(test.huga) is [\\[]!%]\nSUCCESS!"
Copy link
Member Author

@mkurz mkurz Apr 11, 2024

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:

error < > was unexpected at this time.

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:

  1. You have to be on Windows
  2. Checkout the current main branch
  3. Go to the failing scripted test's directory:
cd src/sbt-test/windows/test-bat-template
  1. The test succeeds with sbt 1.5.8:
sbt --sbt-version 1.5.8 -Dproject.version=1.9.16
sbt:windows-test> stage
sbt:windows-test> checkScript
# The test works
  1. The test fails starting with sbt 1.6.0-M1:
sbt --sbt-version 1.6.0-M1 -Dproject.version=1.9.16
sbt:windows-test> stage
sbt:windows-test> checkScript
# The test fails with above error message

I 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 the TrapExit.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).

Copy link
Collaborator

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, TIL.

Yeah, TIL too, took me hours to figure out what the hell is going on...

)

// can't success include double-quote. arguments pass from Process(Seq("-Da=xx\"yy", "aa\"bb")) is parsed (%1="-Da", %2="xx\"yy aa\"bb") by cmd.exe ...
Expand Down
Loading