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

fix: Fixes infinite loop in auto install not found bash function #4094

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

bnorick
Copy link
Contributor

@bnorick bnorick commented Jan 14, 2025

This infinite loop bug is triggered when mise itself becomes not found, e.g., by deleting the directory mise is in.

Here's some output of the shell with set -x on when doing rm -rf on the directory (workspace/) containing mise in some subpath:

+ rm -rf workspace/
++ _mise_hook
++ local previous_exit_status=0
+++ mise hook-env -s bash
+++ local command
+++ command=hook-env
+++ '[' 3 = 0 ']'
+++ shift
+++ case "$command" in
+++ command mise hook-env -s bash
+++ mise hook-not-found -s bash -- mise
+++ local command
+++ command=hook-not-found
+++ '[' 5 = 0 ']'
+++ shift
+++ case "$command" in
+++ command mise hook-not-found -s bash -- mise
+++ mise hook-not-found -s bash -- mise
+++ local command
+++ command=hook-not-found
+++ '[' 5 = 0 ']'
+++ shift
+++ case "$command" in
+++ command mise hook-not-found -s bash -- mise
+++ mise hook-not-found -s bash -- mise
+++ local command
+++ command=hook-not-found
+++ '[' 5 = 0 ']'
+++ shift
+++ case "$command" in
+++ command mise hook-not-found -s bash -- mise
+++ mise hook-not-found -s bash -- mise
+++ local command
+++ command=hook-not-found
+++ '[' 5 = 0 ']'
+++ shift
+++ case "$command" in
+++ command mise hook-not-found -s bash -- mise
+++ mise hook-not-found -s bash -- mise

This infinite loop bug is triggered when mise itself becomes not found, e.g., by deleting the directory mise is in.
@bnorick bnorick changed the title Fixes infinite loop in auto install not found bash function fix: Fixes infinite loop in auto install not found bash function Jan 14, 2025
@bnorick
Copy link
Contributor Author

bnorick commented Jan 14, 2025

Tests failed because I didn't update the test snap, I did that now. However, this made me realize that this'll need fixing for the other shells too, which I am not proficient in.

@bnorick
Copy link
Contributor Author

bnorick commented Jan 14, 2025

The remaining test failure, related to docker-compose seems unrelated to this? Not sure what is going on there...

 mise [email protected]  install
  mise [email protected]  download docker-compose-linux-x86_64
  mise [email protected]  checksum docker-compose-linux-x86_64
  mise [email protected]  extract docker-compose-linux-x86_64
  mise [email protected] ✓ installed
  mise $ /home/runner/.local/share/mise/installs/docker-compose/2.32.3/docker-cli-plugin-docker-compose --version
  Docker Compose version 083f676
  mise ERROR docker-compose: 
     0: expected output not found: Docker Compose version v2.32.3, got: Docker Compose version 083f676

@@ -71,7 +71,7 @@ impl Shell for Bash {
fi

command_not_found_handle() {{
if {exe} hook-not-found -s bash -- "$1"; then
if [ "$1" != "mise" ] && {exe} hook-not-found -s bash -- "$1"; then
Copy link
Owner

Choose a reason for hiding this comment

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

this should match this logic

&& (*env::MISE_BIN_NAME == mise_bin || env::MISE_BIN_NAME.starts_with("mise-"))
since "mise-foo" is a valid mise name

@jdx
Copy link
Owner

jdx commented Jan 14, 2025

I'd like at least support for zsh (which should be close to bash), fish would also be quite nice but the others I'm not so concerned about, I don't even think most of them have not_found handlers

@bnorick
Copy link
Contributor Author

bnorick commented Jan 15, 2025

I didn't check whether the zsh or fish implementations work and they were guided by an LLM. Any help here would be appreciated.

@jdx jdx merged commit fcbe2c4 into jdx:main Jan 15, 2025
18 checks passed
@bnorick
Copy link
Contributor Author

bnorick commented Jan 15, 2025

I am assuming either the tests cover this enough or you tested a bit 🤣

halostatue added a commit to halostatue/mise that referenced this pull request Jan 17, 2025
An error for `fish_command_not_found` was introduce in jdx#4094 because
glob misses do not work the same in fish as in other shells. As such,
we are getting the following error on an unknown function:

```
- (line 64): No matches for wildcard '"mise-"*'. See `help wildcards-globbing`.
    if test "$argv[1]" != "mise" -a "$argv[1]" != "mise-"*
```

This has been replaced with a fish-specific implementation test that
prevents the error and is somewhat more idiomatic as well.
jdx pushed a commit that referenced this pull request Jan 17, 2025
An error for `fish_command_not_found` was introduce in #4094 because
glob misses do not work the same in fish as in other shells. As such,
we are getting the following error on an unknown function:

```
- (line 64): No matches for wildcard '"mise-"*'. See `help wildcards-globbing`.
    if test "$argv[1]" != "mise" -a "$argv[1]" != "mise-"*
```

This has been replaced with a fish-specific implementation test that
prevents the error and is somewhat more idiomatic as well.
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.

2 participants