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

SdkMan per-shell settings do not work #188

Closed
gnodet opened this issue Dec 22, 2023 · 1 comment
Closed

SdkMan per-shell settings do not work #188

gnodet opened this issue Dec 22, 2023 · 1 comment
Assignees
Labels
bug Something isn't working legacy Issue pertaining to WaveLegacy
Milestone

Comments

@gnodet
Copy link

gnodet commented Dec 22, 2023

Describe the bug
SdkMan provides a simple way to switch between various versions of some programs. The settings can be global or per-shell. The per-shell settings seems to be lost of the command is executed.

To Reproduce
Steps to reproduce the behavior:

  1. Install SDK:
curl -s "https://get.sdkman.io" | bash
source "$HOME/.sdkman/bin/sdkman-init.sh"
  1. Install two different versions of maven
sdk install maven 3.8.8
sdk install maven 3.9.6
  1. Use 3.9.6 as the default
sdk default maven 3.9.6
  1. Verify selecting a different version in the current the shell
(sdk use maven 3.8.5 && mvn -v)

The output contains Apache Maven 3.8.5 (3599d3414f046de2324203b78ddcf9b5e4388aa0)
5. Verify selecting a different version which should persist

sdk use maven 3.8.5 

and later

mvn -v

The output contains Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae)

Expected behavior
The sdkman shell settings should be persisted in the session.

Analysis
It seems that SdkMan directly changes the PATH variable to point to the correct tool version, see https://github.com/sdkman/sdkman-cli/blob/master/src/main/bash/sdkman-use.sh#L49

@sawka sawka added the bug Something isn't working label Dec 22, 2023
@sawka
Copy link
Member

sawka commented Dec 22, 2023

Thanks for submitting.

As a workaround you can add "[rtnstate=1]" to the beginning of your command and it should persist the state. e.g. [rtnstate=1] (sdk use maven 3.8.5 && mvn -v).

We'll also look at getting this supported directly in Wave without the extra rtnstate annotation.

@sawka sawka added this to the v0.6.0 milestone Dec 22, 2023
@sawka sawka self-assigned this Dec 22, 2023
sawka added a commit that referenced this issue Jan 17, 2024
adds zsh support to waveterm.  big change, lots going on here.  lots of other improvements and bug fixes added while debugging and building out the feature.

Commits:

* refactor shexec parser.go into new package shellenv.  separate out bash specific parsing from generic functions

* checkpoint

* work on refactoring shexec.  created two new packages shellapi (for bash/zsh specific stuff), and shellutil (shared between shellapi and shexec)

* more refactoring

* create shellapi interface to abstract bash specific functionality

* more refactoring, move bash shell state parsing to shellapi

* move makeRcFile to shellapi.  remove all of the 'client' options CLI options from waveshell

* get shellType passed through to server/single paths for waveshell

* add a local shelltype detector

* mock out a zshapi

* move shelltype through more of the code

* get a command to run via zsh

* zsh can now switch directories.  poc, needs cleanup

* working on ShellState encoding differences between zsh/bash.  Working on parsing zsh decls.  move utilfn package into waveshell (shouldn't have been in wavesrv)

* switch to use []byte for vardecl serialization + diffs

* progress on zsh environment.  still have issues reconciling init environment with trap environment

* fix typeset argument parsing

* parse promptvars, more zsh specific ignores

* fix bug with promptvar not getting set (wrong check in FeState func)

* add sdk (issue #188) to list of rtnstate commands

* more zsh compatibility -- working with a larger ohmyzsh environment.  ignore more variables, handle exit trap better.  unique path/fpath.  add a processtype variable to base.

* must return a value

* zsh alias parsing/restoring.  diff changes (and rtnstate changes).  introduces linediff v1.

* force zmodload of zsh/parameter

* starting work on zsh functions

* need a v1 of mapdiff as well (to handle null chars)

* pack/unpack of ints was wrong (one used int and one use uint).  turned out we only ever encoded '0' so it worked.  that also means it is safe to change unpack to unpackUInt

* reworking for binary encoding of aliases and functions (because of zsh allows any character, including nulls, in names and values)

* fixes, working on functions, issue with line endings

* zsh functions.  lots of ugliness here around dealing with line dicipline and cooked stty.  new runcommand function to grab output from a non-tty fd.  note that we still to run the actual command in a stty to get the proper output.

* write uuid tempdir, cleanup with tmprcfilename code

* hack in some simple zsh function declaration finding code for rtnstate.  create function diff for rtnstate that supports zsh

* make sure key order is constant so shell hashes are consistent

* fix problems with state diffs to support new zsh formats.  add diff/apply code to shellapi (moved from shellenv), that is now specific to zsh or bash

* add log packet and new shellstate packets

* switch to shellstate map that's also keyed by shelltype

* add shelltype to remoteinstance

* remove shell argument from waveshell

* added new shelltype statemap to remote.go (msh), deal with fallout

* move shellstate out of init packet, and move to an explicit reinit call.  try to initialize all of the active shell states

* change dont always store init state (only store on demand).  initialize shell states on demand (if not already initialized).  allow reset to change shells

* add shellpref field to remote table.  use to drive the default shell choice for new tabs

* show shelltag on cmdinput, pass through ri and remote (defaultshellstate)

* bump mshell version to v0.4

* better version validation for shellstate.  also relax compatibility requirements for diffing states (shelltype + major version need to match)

* better error handling, check shellstate compatibility during run (on waveshell server)

* add extra separator for bash shellstate processing to deal with spurious output from rc files

* special migration for v30 -- flag invalid bash shell states and show special button in UI to fix

* format

* remove zsh-decls (unused)

* remove test code

* remove debug print

* fix typo
@sawka sawka closed this as completed Mar 7, 2024
@esimkowitz esimkowitz added the legacy Issue pertaining to WaveLegacy label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working legacy Issue pertaining to WaveLegacy
Projects
None yet
Development

No branches or pull requests

3 participants