-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(backend): allow upgrading vfox backend tools with symlinked installations #7012
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 all commits
d188cd2
25011f3
1c52146
98096b4
54879a2
65b2f1a
3557fe0
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,79 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Test that HTTP backend tools (which use symlinks to cache) can be upgraded | ||
| # This validates the fix for https://github.com/jdx/mise/pull/7012 | ||
| # | ||
| # The HTTP backend creates symlinks from installs/ to http-tarballs/ cache. | ||
| # Before the fix, these symlinks caused tools to be incorrectly skipped | ||
| # in the outdated check, preventing upgrades from working. | ||
|
|
||
| # Start a simple HTTP server to serve version list | ||
| VERSION_LIST_DIR="$TMPDIR/version-list-server" | ||
| mkdir -p "$VERSION_LIST_DIR" | ||
|
|
||
| # Create version list file with two versions | ||
| echo -e "1.0.0\n2.0.0" >"$VERSION_LIST_DIR/versions.txt" | ||
|
|
||
| # Start Python HTTP server in background | ||
| (cd "$VERSION_LIST_DIR" && python3 -m http.server 18123 &>/dev/null) & | ||
| HTTP_SERVER_PID=$! | ||
|
|
||
| # Wait for server to start | ||
| sleep 1 | ||
|
|
||
| # Cleanup function | ||
| cleanup() { | ||
| kill $HTTP_SERVER_PID 2>/dev/null || true | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| # Verify server is running | ||
| if ! curl -s http://localhost:18123/versions.txt | grep -q "1.0.0"; then | ||
| fail "HTTP server failed to start" | ||
| fi | ||
|
|
||
| # Test 1: Install HTTP tool and verify it's a symlink to cache | ||
| echo "Test 1: Install HTTP tool with version_list_url" | ||
| cat <<EOF >mise.toml | ||
| [tools."http:hello-upgrade"] | ||
| version = "1.0.0" | ||
| url = "https://mise.jdx.dev/test-fixtures/hello-world-{{version}}.tar.gz" | ||
| version_list_url = "http://localhost:18123/versions.txt" | ||
| bin_path = "hello-world-1.0.0/bin" | ||
| postinstall = "chmod +x \$MISE_TOOL_INSTALL_PATH/hello-world-1.0.0/bin/hello-world" | ||
| EOF | ||
|
|
||
| mise install | ||
| assert_contains "mise x -- hello-world" "hello world" | ||
|
|
||
| # Verify install is a symlink (HTTP backend caching behavior) | ||
| INSTALL_PATH="$MISE_DATA_DIR/installs/http-hello-upgrade/1.0.0" | ||
| if [[ ! -L $INSTALL_PATH ]]; then | ||
| fail "Expected install path to be a symlink: $INSTALL_PATH" | ||
| fi | ||
| echo "Verified: Install path is a symlink (HTTP backend caching)" | ||
|
|
||
| # Test 2: Verify mise latest sees the newer version | ||
| echo "Test 2: Check that mise latest sees version 2.0.0" | ||
| assert "mise latest http:hello-upgrade" "2.0.0" | ||
|
|
||
| # Test 3: Verify mise outdated --bump shows the tool as outdated | ||
| # This is the key test - before the fix, HTTP backend tools were skipped | ||
| echo "Test 3: Check that mise outdated --bump detects outdated version" | ||
| OUTDATED_OUTPUT=$(mise outdated --bump 2>&1) | ||
| if [[ $OUTDATED_OUTPUT != *"hello-upgrade"* ]] || [[ $OUTDATED_OUTPUT != *"2.0.0"* ]]; then | ||
| echo "outdated output: $OUTDATED_OUTPUT" | ||
| fail "mise outdated --bump should show http:hello-upgrade as outdated to 2.0.0" | ||
| fi | ||
| echo "Verified: mise outdated --bump correctly detects HTTP backend tool as outdated" | ||
|
|
||
| # Test 4: Verify mise upgrade --bump --dry-run shows the upgrade | ||
| echo "Test 4: Check that mise upgrade --bump --dry-run shows the upgrade" | ||
| UPGRADE_OUTPUT=$(mise upgrade --bump --dry-run 2>&1) | ||
| if [[ $UPGRADE_OUTPUT != *"Would install"* ]] || [[ $UPGRADE_OUTPUT != *"hello-upgrade"* ]]; then | ||
| echo "upgrade output: $UPGRADE_OUTPUT" | ||
| fail "mise upgrade --bump --dry-run should show install for http:hello-upgrade" | ||
| fi | ||
| echo "Verified: mise upgrade --bump --dry-run correctly shows upgrade for HTTP backend tool" | ||
|
|
||
| echo "SUCCESS: HTTP backend upgrade test passed!" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -542,10 +542,22 @@ pub trait Backend: Debug + Send + Sync { | |
| !self.is_version_installed(config, tv, true) || is_outdated_version(&tv.version, &latest) | ||
| } | ||
| fn symlink_path(&self, tv: &ToolVersion) -> Option<PathBuf> { | ||
| match tv.install_path() { | ||
| path if path.is_symlink() && !is_runtime_symlink(&path) => Some(path), | ||
| _ => None, | ||
| let path = tv.install_path(); | ||
| if !path.is_symlink() { | ||
| return None; | ||
| } | ||
| // Only skip symlinks pointing within installs (user aliases, not backend-managed) | ||
| if let Ok(Some(target)) = file::resolve_symlink(&path) { | ||
| let target = if target.is_absolute() { | ||
| target | ||
| } else { | ||
| path.parent().unwrap_or(&path).join(&target) | ||
| }; | ||
| if target.starts_with(*dirs::INSTALLS) { | ||
| return Some(path); | ||
| } | ||
|
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. Non-canonicalized paths can bypass symlink checkLow Severity The |
||
| } | ||
| None | ||
| } | ||
| fn create_symlink(&self, version: &str, target: &Path) -> Result<Option<(PathBuf, PathBuf)>> { | ||
| let link = self.ba().installs_path.join(version); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why didn't this cover this situation to begin with? seems like we're just reimplementing is_runtime_symlink
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that
is_runtime_symlinkchecks if the symlink target starts with./. It identifies mise's internal version alias symlinks (like ./20.18.0), so the original logic !is_runtime_symlink means "anything that's not a ./ symlink."Nix backend symlinks point to /nix/store/..., which doesn't start with ./, so is_runtime_symlink returns false, and !false = true results in returns Some(path) which skips upgrade.
So somehow we have to look if a symlink points within installs.
We could perhaps modify is_runtime_symlink directly instead. Something like:
An edge case with that approach could be if a user manually creates an absolute symlink pointing within installs (myalias → /home/user/.local/share/mise/installs/node/20.18.0), the existing implementation doesn't treat that as a mise-created symlink, but the new code would treat that same user-created absolute path symlink as a mise-created symlink and so might delete it during a clean up.