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

Add LIMA_SHELL env var to lima command #780

Merged
merged 1 commit into from
Apr 8, 2022

Conversation

jandubois
Copy link
Member

@jandubois jandubois commented Apr 6, 2022

$ export LIMA_INSTANCE=alpine
$ lima sudo apk add fish
[...]
$ export LIMA_SHELL=/usr/bin/fish
$ lima
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
jan@lima-alpine /U/jan>

Split off from #779.

@jandubois jandubois added this to the v0.10.0 milestone Apr 6, 2022
@@ -1,6 +1,7 @@
#!/bin/sh
set -eu
: "${LIMA_INSTANCE:=default}"
: "${LIMA_SHELL:=}"
Copy link
Member

Choose a reason for hiding this comment

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

Wondering rather we should have a generic way to support limactl shell options in the lima script. Perhaps like LIMA_SHELL_ARGS=--workdir=/ --shell=/bin/zsh. But on the second thought maybe users should just set alias lima=limactl shell .....

Originally posted by @AkihiroSuda in #779 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

should have a generic way to support limactl shell options in the lima script

Practically speaking, we only have 2 options: --shell and --workdir, and I don't really expect somebody to hard-code the workdir for each lima call. But even then, I would rather add another LIMA_WORKDIR variable instead, so the options can be manipulated independent of each other.

Copy link
Member

Choose a reason for hiding this comment

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

we only have 2 options

Eventually we will have more

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but I still like having individual variables. This is how I would do it:

set - "$LIMA_INSTANCE" "$@"
if [ -n "${LIMA_SHELL}" ]; then
  set - --shell "$LIMA_SHELL" "$@"
fi
if [ -n "${LIMA_WORKDIR}" ]; then
  set - --workdir "$LIMA_WORKDIR" "$@"
fi
exec "$LIMACTL" shell "$@"

This is trivially extensible to additional settings.

Copy link
Member Author

@jandubois jandubois Apr 6, 2022

Choose a reason for hiding this comment

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

I've updated the PR to include support for LIMA_WORKDIR as well (and rebased on top of master, so the --shell option is actually functional.

To override the default shell configured inside the instance.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
@AkihiroSuda
Copy link
Member

CI failing

Warning: Attempt 1 failed. Reason: Child_process exited with error code 1

bash: line 1: ./hack/test-upgrade.sh: No such file or directory
Warning: Attempt 2 failed. Reason: Child_process exited with error code 127

bash: line 1: ./hack/test-upgrade.sh: No such file or directory
Error: Final attempt failed. Child_process exited with error code 127

https://github.com/lima-vm/lima/runs/5864063545?check_suite_focus=true

@jandubois
Copy link
Member Author

CI failing

Seems like CI flakiness; it passed on re-run.

@jandubois jandubois merged commit 7c8e81c into lima-vm:master Apr 8, 2022
@jandubois jandubois deleted the lima branch April 8, 2022 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants