-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(install): suppress spurious dependency warning when tool is configured #8923
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
Changes from 1 commit
4689ec9
d6204d2
60c52ed
882ba4c
93771db
5183085
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Test that "npm may be required" warning is NOT shown when node is configured | ||
| # in the same mise.toml alongside an npm package (even if node isn't installed yet). | ||
| # Regression test for https://github.com/jdx/mise/discussions/8919 | ||
|
|
||
| # Create a PATH that has mise but not npm/node | ||
| MISE_BIN="$(command -v mise)" | ||
| MISE_DIR="$(dirname "$MISE_BIN")" | ||
| export PATH="$MISE_DIR:/usr/bin:/bin:/usr/sbin:/sbin" | ||
|
|
||
| # Verify npm is not on PATH | ||
| if command -v npm >/dev/null 2>&1; then | ||
| echo "SKIP: npm is already in PATH, cannot test missing dependency behavior" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Create a mise.toml that has both node and an npm package | ||
| cat >mise.toml <<'EOF' | ||
| [tools] | ||
| node = "latest" | ||
| "npm:prettier" = "3" | ||
| EOF | ||
|
|
||
| # Run ls-remote for the npm package — should NOT show the warning since node is configured | ||
| output=$(MISE_LOG_LEVEL=warn mise ls-remote npm:prettier 2>&1 || true) | ||
|
|
||
| if [[ $output == *"npm may be required but was not found"* ]]; then | ||
| echo "FAIL: warning was shown even though node is configured in mise.toml" | ||
| echo "Output: $output" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "PASS: no spurious npm warning when node is configured" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1365,6 +1365,14 @@ pub trait Backend: Debug + Send + Sync { | |
| }; | ||
|
|
||
| if !found { | ||
| // Check if a tool providing this program is configured in the toolset | ||
| // (even if not yet installed). If so, mise will install it as a dependency | ||
| // before this tool needs it, so the warning is spurious. | ||
| if let Ok(ts) = self.dependency_toolset(config).await { | ||
| if !ts.list_current_versions().is_empty() { | ||
| return; | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation suppresses the warning if any dependency of the backend is configured in the toolset. This is too broad and can lead to missing warnings for unrelated dependencies. For example, if a backend depends on both Additionally, Consider making the check more specific to the if let Ok(ts) = self.dependency_toolset(config).await {
let is_configured = ts.list_current_versions().iter().any(|(b, _)| {
b.id() == program || b.tool_name() == program ||
REGISTRY.get(program).is_some_and(|rt| {
rt.overrides.iter().any(|o| *o == b.id() || *o == b.tool_name())
})
});
if is_configured {
return;
}
}
cursor[bot] marked this conversation as resolved.
|
||
| warn!( | ||
| "{} may be required but was not found.\n\n{}", | ||
| program, install_instructions | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.