Skip to content

[1.x] fix: --allow-empty fixes #7966

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 22, 2024
Merged

[1.x] fix: --allow-empty fixes #7966

merged 1 commit into from
Dec 22, 2024

Conversation

eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented Dec 20, 2024 •
edited
Loading

Fixes #7942
Fixes #2123

Problem

  1. Currently users are automatically opted into -create-sbt, somewhat implicitly.
  2. When somehow they are not, the check mechanism currently blocks for input.
  3. The working directory check is invoked even for sbt --script-version

Solution

  1. Support a new location for sbtopts file under $XDG_CONFIG_HOME/sbt
  2. Rename -create-sbt to --allow-empty, and don't opt everyone in
  3. Run the working directory check only when sbt is about to start a session
  4. Exit 1 instead of blocking for input

@eed3si9n eed3si9n mentioned this pull request Dec 20, 2024
Closed
francisdb
francisdb reviewed Dec 20, 2024
View reviewed changes
eed3si9n
eed3si9n commented Dec 20, 2024
View reviewed changes
sbt
@@ -519,6 +544,7 @@ run() {
done
echo "shutdown ${#sbt_processes[@]} sbt processes"
else
checkWorkingDirectory
Copy link
Member Author

Choose a reason for hiding this comment

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

checkWorkingDirectory is deferred until here.

Friendseeker
Friendseeker reviewed Dec 20, 2024
View reviewed changes
@Friendseeker
Copy link
Member

Friendseeker commented Dec 20, 2024 •
edited
Loading

Tested on Windows (Windows 10, Windows Terminal with Windows Powershell). Tried to follow instruction to opt out of --allow-empty check yet error is still thrown.

PS D:\Repos\novel> cat 'C:\Users\Jerry Tan\AppData\Local\sbt\sbtconfig.txt'
--allow-empty
PS D:\Repos\novel>  & 'C:\Users\Jerry Tan\Downloads\sbt2.bat'
[error] Neither build.sbt nor a 'project' directory in the current directory: "D:\Repos\novel"
[error] run 'sbt new', touch build.sbt, or run 'sbt --allow-empty'.
[error]
[error] To opt out of this check, create C:\Users\Jerry Tan\AppData\Local\sbt\sbtconfig.txt with
[error] --allow-empty

@Friendseeker
Copy link
Member

Friendseeker commented Dec 20, 2024 •
edited
Loading

Issue 2: On Windows (same setup as #7966 (comment)), sbt new is broken (sbt2.bat is downloaded from PR branch)

PS D:\Repos\novel> cat 'C:\Users\Jerry Tan\AppData\Local\sbt\sbtconfig.txt'
--allow-empty
PS D:\Repos\novel> & 'C:\Users\Jerry Tan\AppData\Local\Coursier\cache\arc\https\github.com\sbt\sbt\releases\download\v1.10.5\sbt-1.10.5.zip\sbt\bin\sbt2.bat' new
Unrecognized option: --allow-empty
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
The filename, directory name, or volume label syntax is incorrect.
Unrecognized option: --allow-empty
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
Unrecognized option: --allow-empty
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
PS D:\Repos\novel> & 'C:\Users\Jerry Tan\AppData\Local\Coursier\cache\arc\https\github.com\sbt\sbt\releases\download\v1.10.5\sbt-1.10.5.zip\sbt\bin\sbt.bat' new

Welcome to sbt new!
Here are some templates to get started:
 a) scala/toolkit.local               - Scala Toolkit (beta) by Scala Center and VirtusLab
 b) typelevel/toolkit.local           - Toolkit to start building Typelevel apps
 c) sbt/cross-platform.local          - A cross-JVM/JS/Native project
 d) scala/scala3.g8                   - Scala 3 seed template
 e) scala/scala-seed.g8               - Scala 2 seed template
 f) playframework/play-scala-seed.g8  - A Play project in Scala
 g) playframework/play-java-seed.g8   - A Play project in Java
 i) softwaremill/tapir.g8             - A tapir project using Netty
 m) scala-js/vite.g8                  - A Scala.JS + Vite project
 n) holdenk/sparkProjectTemplate.g8   - A Scala Spark project
 o) spotify/scio.g8                   - A Scio project
 p) disneystreaming/smithy4s.g8       - A Smithy4s project
 q) quit
Select a template: Terminate batch job (Y/N)? Y

I guess --allow-empty is somehow... passed as a java arg? And this is fundamentally the same issue as #7966 (comment)

Friendseeker
Friendseeker reviewed Dec 20, 2024
View reviewed changes
@Friendseeker
Copy link
Member

Test environment: Windows 10, Windows Terminal with Git Bash

Jerry Tan@DESKTOP-6SUJJE2 MINGW64 ~/empty
$ ls

Jerry Tan@DESKTOP-6SUJJE2 MINGW64 ~/empty
$ sbt2
[warn] No sbt.version set in project/build.properties, base directory: C:\Users\Jerry Tan\empty
[info] welcome to sbt 1.10.6 (Oracle Corporation Java 17.0.6)
[info] set current project to empty (in build file:/C:/Users/Jerry%20Tan/empty/)
[info] sbt server started at local:sbt-server-4c79a3e93794150e9089
[info] started sbt server
sbt:empty> shutdown
[info] shutting down sbt server

Note we did not append --allow-empty, yet we still launched sbt in an empty directory.

@eed3si9n
Copy link
Member Author

I pulled out my old IdeaPad, and it seems to work for me on both Command Prompt and Git Bash:

windows

@Friendseeker
Copy link
Member

I pulled out my old IdeaPad, and it seems to work for me on both Command Prompt and Git Bash:

windows

Interesting. I am now away from my Windows computer, shall try to reproduce the issue once I got back to it.

@eed3si9n
--allow-empty fixes
cd58481 
**Problem**
1. Currently users are automatically opted into -create-sbt,
   somewhat implicitly.
2. When somehow they are not, the check mechanism currently blocks for input.

**Solution**
1. Support a new location for sbtopts file under XSG_CONFIG_HOME/sbt
2. Rename -create-sbt to --allow-empty, and don't opt everyone in
3. Exit 1 instead of blocking for input
@eed3si9n
Copy link
Member Author

In the interest of time, I plan to squash and land this today.

@Friendseeker
Copy link
Member

In the interest of time, I plan to squash and land this today.

Yeah let's do that for now. We can investigate #7966 (comment) later. I won't get access to Windows machine for the next few days.

@eed3si9n eed3si9n merged commit bb796dc into sbt:1.10.x Dec 22, 2024
10 checks passed
@eed3si9n eed3si9n deleted the wip/allow-empty branch December 22, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adpi2 adpi2 Awaiting requested review from adpi2

@BillyAutrey BillyAutrey Awaiting requested review from BillyAutrey

2 more reviewers

@francisdb francisdb francisdb left review comments

@Friendseeker Friendseeker Friendseeker left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@eed3si9n @Friendseeker @francisdb