Performing sdk uninstall causes "thread 'main' panicked"

kevin-canadian commented 1 year ago

I am running sdkman in Cygwin to manage different java versions.

When I run sdk uninstall java 17.0.2-tem I get the following output.

thread 'main' panicked at 'panic! the candidates file is missing', src\lib.rs:44:14

If I run RUST_BACKTRACE=full sdk uninstall java 17.0.7-tem I get the following.

thread 'main' panicked at 'panic! the candidates file is missing', src\lib.rs:44:14 stack backtrace: 0: 0x7ff6a76af622 - 1: 0x7ff6a76c2d1b - 2: 0x7ff6a76abdda - 3: 0x7ff6a76af36b - 4: 0x7ff6a76b15f9 - 5: 0x7ff6a76b127b - 6: 0x7ff6a76b1e90 - 7: 0x7ff6a76b1c1e - 8: 0x7ff6a76b003f - 9: 0x7ff6a76b18d0 - 10: 0x7ff6a76c9b05 - 11: 0x7ff6a76c20e0 - 12: 0x7ff6a76c208b - 13: 0x7ff6a76c9ac9 - 14: 0x7ff6a764e4e4 - 15: 0x7ff6a7647136 - 16: 0x7ff6a764b456 - 17: 0x7ff6a764c2fc - 18: 0x7ff6a76a6b7e - 19: 0x7ff6a764a2dc - 20: 0x7ff6a76c7a90 - 21: 0x7ff9441f7614 - BaseThreadInitThunk 22: 0x7ff9454a26a1 - RtlUserThreadStart

I don't know when this started since this is the first time I've tried to uninstall a version.

Before attempting the uninstall I had just upgraded to the last version (SDKMAN_VERSION="5.18.1" and SDKMAN_NATIVE_VERSION="0.2.2", according to the current content at https://get.sdkman.io/). I also tried sdk selfupdate force, and started a new terminal, to no avail.

Perhaps a related issue: running sdk version (or RUST_BACKTRACE=full sdk version) returns no output at all (process exit code is 78).

marc0der commented 1 year ago

Hi @kevin-canadian, Thanks for reporting this issue. Could I please ask you to run the following?

ls -l "$SDKMAN_DIR/var"
cat "$SDKMAN_DIR/var/candidates"

I'd like to determine if the candidates file really is missing, or if the new native uninstall is not finding it correctly.

kevin-canadian commented 1 year ago

$ ls -l "$SDKMAN_DIR/var"
total 5
drwx------+ 1 kele domain-users   0 Apr 21 21:39 metadata/
-rw-------+ 1 kele domain-users  78 Feb 28 09:48 broadcast
-rw-------+ 1 kele domain-users  25 Feb 28 09:48 broadcast_id
-rw-------+ 1 kele domain-users 508 Apr 21 21:44 candidates
-rw-------+ 1 kele domain-users   0 Apr 21 21:33 delay_upgrade
-rw-------+ 1 kele domain-users   7 Apr 21 21:44 version
-rw-------+ 1 kele domain-users   6 Apr 21 21:44 version_native

$ cat "$SDKMAN_DIR/var/candidates"
activemq,ant,asciidoctorj,ballerina,bld,bpipe,btrace,concurnas,connor,coursier,cuba,cxf,doctoolchain,flink,gaiden,gradle,gradleprofiler,grails,groovy,groovyserv,hadoop,helidon,http4k,infrastructor,jarviz,java,jbake,jbang,jmc,jmeter,joern,jreleaser,karaf,kcctl,ki,kobweb,kotlin,kscript,ktx,layrry,leiningen,maven,mcs,micronaut,mulefd,mvnd,mybatis,neo4jmigrations,pierrot,pomchecker,quarkus,sbt,scala,scalacli,schemacrawler,skeletal,spark,springboot,sshoogr,taxi,test,tomcat,toolkit,vertx,visualvm,webtau,znai

$ ls -al $SDKMAN_DIR/candidates
total 8
drwx------+ 1 kele domain-users 0 Mar  3  2022 ./
drwx------+ 1 kele domain-users 0 Apr 21 21:44 ../
drwx------+ 1 kele domain-users 0 Apr 21 21:38 java/

$ ls -al $SDKMAN_DIR/candidates/java
total 24
drwx------+ 1 kele domain-users  0 Apr 21 21:38 ./
drwx------+ 1 kele domain-users  0 Mar  3  2022 ../
drwxrwx---+ 1 kele domain-users  0 Jan 18  2022 11.0.14-tem/
drwxrwxr-x+ 1 kele domain-users  0 Jan 20  2022 17.0.2-tem/
drwxrwxr-x+ 1 kele domain-users  0 Apr 20  2022 17.0.3-tem/
drwxrwxr-x+ 1 kele domain-users  0 Apr 19 00:34 17.0.7-tem/
drwxrwxr-x+ 1 kele domain-users  0 Jan 19  2022 8.0.322-tem/
lrwxrwxrwx  1 kele domain-users 10 Apr 21 21:38 current -> 17.0.7-tem/

$ env | grep SDKMAN
SDKMAN_CANDIDATES_API=https://api.sdkman.io/2
SDKMAN_DIR=/home/kele/.sdkman
SDKMAN_CANDIDATES_DIR=/home/kele/.sdkman/candidates
SDKMAN_VERSION=5.16.0
SDKMAN_PLATFORM=cygwin_nt-10.0-19045

$ set | egrep '^SDKMAN'
SDKMAN_CANDIDATES=([0]="activemq" [1]="ant" [2]="asciidoctorj" [3]="ballerina" [4]="bpipe" [5]="btrace" [6]="concurnas" [7]="connor" [8]="cuba" [9]="cxf" [10]="doctoolchain" [11]="flink" [12]="gaiden" [13]="gradle" [14]="gradleprofiler" [15]="grails" [16]="groovy" [17]="groovyserv" [18]="hadoop" [19]="http4k" [20]="infrastructor" [21]="java" [22]="jbake" [23]="jbang" [24]="jmc" [25]="jmeter" [26]="joern" [27]="jreleaser" [28]="karaf" [29]="kcctl" [30]="ki" [31]="kobweb" [32]="kotlin" [33]="kscript" [34]="layrry" [35]="leiningen" [36]="maven" [37]="mcs" [38]="micronaut" [39]="mulefd" [40]="mvnd" [41]="mybatis" [42]="neo4jmigrations" [43]="pierrot" [44]="pomchecker" [45]="quarkus" [46]="sbt" 
[47]="scala" [48]="scalacli" [49]="schemacrawler" [50]="spark" [51]="springboot" [52]="sshoogr" [53]="taxi" [54]="test" [55]="tomcat" [56]="toolkit" [57]="vertx" [58]="visualvm" [59]="webtau" [60]="znai")
SDKMAN_CANDIDATES_API=https://api.sdkman.io/2
SDKMAN_CANDIDATES_CACHE=/home/kele/.sdkman/var/candidates
SDKMAN_CANDIDATES_CSV=activemq,ant,asciidoctorj,ballerina,bpipe,btrace,concurnas,connor,cuba,cxf,doctoolchain,flink,gaiden,gradle,gradleprofiler,grails,groovy,groovyserv,hadoop,http4k,infrastructor,java,jbake,jbang,jmc,jmeter,joern,jreleaser,karaf,kcctl,ki,kobweb,kotlin,kscript,layrry,leiningen,maven,mcs,micronaut,mulefd,mvnd,mybatis,neo4jmigrations,pierrot,pomchecker,quarkus,sbt,scala,scalacli,schemacrawler,spark,springboot,sshoogr,taxi,test,tomcat,toolkit,vertx,visualvm,webtau,znai
SDKMAN_CANDIDATES_DIR=/home/kele/.sdkman/candidates
SDKMAN_DIR=/home/kele/.sdkman
SDKMAN_KERNEL=CYGWIN_NT-10.0-19045
SDKMAN_PLATFORM=cygwin_nt-10.0-19045
SDKMAN_VERSION=5.16.0

marc0der commented 1 year ago

Great, thanks for all the info! I'll try to replicate this in a VM to see what's going on. BTW, a temporary workaround is to delete the uninstall command from the libexec folder. Once this is removed, you will automatically fall back to legacy bash command.

kevin-canadian commented 1 year ago

Thanks for the workaround, that worked. Incidentally, I also removed the "version" binary from that folder, and version works now as well. As I said, that one was also broken. I also tried the other commands I found there (without removing them from the libexec folder).

sdk help: works, but output looks funny (ANSI escape sequences displayed).
sdk home: works if input is invalid (i.e. no arguments shows usage info), but panics if arguments are provided correctly.
sdk broadcast: I get a message saying it's been removed. With ANSI escape sequences displayed.

I'm a big bash fan. I think this speaks to the difficulty of making robust cross-platform binary applications, and the benefits of using scripts within a POSIX style environment. :)

marc0der commented 1 year ago

Hey @kevin-canadian,

I've finally had some time to investigate all your points. I installed Cygwin and Git Bash on my Windows gaming machine, and here are my observations:

ANSI escape sequences displayed: I'm not seeing any escape sequences on my machine, which makes me think this is down to some terminal settings on your machine.
File handling: Rust does not play nicely with Cygwin. Rust infers the Windows platform, but Cygwin seems to have a mind of its own and tries to force POSIX rules onto the platform. Git Bash (MSYS) seems saner and handles our native binaries just fine by honouring the Windows platform. Since only 0.13 % of our users are using Git Bash and Cygwin, most of which are on Git Bash, it makes sense to pull native support for Cygwin. It doesn't mean that SDKMAN will stop working on Cygwin, but rather that you won't benefit from the native binaries that we provide going forward. Further, in time we will sunset the bash fallback commands and only support Git Bash (and WSL) on Windows.
Broadcast: This command was removed from SDKMAN as it was rarely used and provided little benefit. We put it to the vote, and most people were indifferent to the removal of this command. This also allowed us to retire one of our legacy microservices.

I'm a big bash fan. I think this speaks to the difficulty of making robust cross-platform binary applications, and the benefits of using scripts within a POSIX style environment. :)

As the maintainer of a fair-sized bash project for eleven years running, I certainly do not share your love for bash :smile: Bash does not guarantee the (bash) version running on your machine. It gives no guarantees of the versions of required transitive dependencies. The scripts require the code to comply with all shells, including ZSH, sh, and exotic ones like fish. It does not have a type system, relegating everything to strings. Arrays are a nightmare, especially across shells. It is interpreted and not compiled, complex to test, inefficient and brittle. Need I say more? :laughing:

A native language, on the other hand, gives you many advantages. Compiled, robust, testable, typesafe code is up there for me. I have guarantees about statically linked version dependencies and don't need to worry about shell compatibility. Yes, we will undergo a period of teething pains, but in the long run, it will make things better and safer for everyone using our tool.

My honest suggestion would be to try out WSL, or if that isn't possible, to give Git Bash a go. It also has some niggles (like symlink support), but it does seem to be what people prefer using over Cygwin these days. I'll certainly be supporting Git Bash in the foreseeable future.

kevin-canadian commented 1 year ago

Thanks for the feedback. This is more or less what I suspected.

ANSI escape sequences: Yes, this is likely due to a setting in the terminal program or an environment variable picked up by Rust. Every now and then we run into programs which have issues figuring out how to deal with escape sequences. The mapping between Windows and *nix TTYs is likely the problem. This is not a major issue either way. I'm sure I could find some settings or environment variables that would resolve this if I looked into it.

File handling: We have similar issues with Golang binaries in Cygwin. Cygwin does indeed use POSIX file paths. We can use binaries compiled for Cygwin (and provide POSIX paths), or binaries compiled for Windows (and either map these to Windows paths or just use raw filenames with no paths). I guess the underlying issue is simply that Rust (and Golang) are getting mixed signals when it comes to where things should be. I understand that we Cygwin users are a dying breed, so removing support is clearly reasonable. We would love to switch to native Linux, but this isn't possible for corporate reasons. We plan to port most of our tools to WSL eventually, but we'll need to spend some time doing this since WSL does strange things with networking within the local machine, and this breaks some of our stuff in strange ways. Tip: watch out for issues with MTU sizes not being respected in WSL.

Broadcast: I only knew it existed because I stumbled on it in one of the directories you pointed to. :)

Regarding bash, of course for large projects bash is often not the best tool. I understand the weaknesses completely (as a bash fan, I have to). The main advantage to which I was referring was portability: i.e. the bash version works in Cygwin (and WSL, and Ubuntu, and...) -- the Rust version does not.

That being said, I understand that as complexity grows, maintainability becomes an issue.

Thanks again for looking into this. Feel free to close (as won't fix).

sdkman / sdkman-cli-native

Performing sdk uninstall causes "thread 'main' panicked" #138