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

Switching from a cargo-home to flat layout breaks updates #210

Open
zanieb opened this issue Nov 8, 2024 · 4 comments
Open

Switching from a cargo-home to flat layout breaks updates #210

zanieb opened this issue Nov 8, 2024 · 4 comments

Comments

@zanieb
Copy link

zanieb commented Nov 8, 2024

In uv 0.4.30, we used the default install path resulting in the following receipt:

{
  "binaries": [
    "uv",
    "uvx"
  ],
  "binary_aliases": {},
  "cdylibs": [],
  "cstaticlibs": [],
  "install_layout": "cargo-home",
  "install_prefix": "/Users/zb/.cargo",
  "modify_path": true,
  "provider": {
    "source": "cargo-dist",
    "version": "0.23.0"
  },
  "source": {
    "app_name": "uv",
    "name": "uv",
    "owner": "astral-sh",
    "release_type": "github"
  },
  "version": "0.4.30"
}

In uv 0.5.0, we switched to a flat layout by specifying target install paths. However, this results in unexpected update behavior. uv self update places the executables in ~/.cargo instead of ~/.cargo/bin. The previous version's executables are not removed. The receipt contains the new version so subsequent update attempts fail.

This appears to be caused by not loading the install_layout from the receipt at

self.current_version_installed_by = Some(provider);
self.install_prefix = Some(receipt.install_prefix);
self.modify_path = receipt.modify_path;

The layout is not passed to the installer and it is hard-coded to flat: https://gist.github.com/zanieb/9dca39f62651087794bbb31fa377d72a#file-uv-installer-sh-L1018-L1020

The update results in the receipt:

{
  "binaries": [
    "uv",
    "uvx"
  ],
  "binary_aliases": {},
  "cdylibs": [],
  "cstaticlibs": [],
  "install_layout": "flat",
  "install_prefix": "/Users/zb/.cargo",
  "modify_path": true,
  "provider": {
    "source": "cargo-dist",
    "version": "0.25.1"
  },
  "source": {
    "app_name": "uv",
    "name": "uv",
    "owner": "astral-sh",
    "release_type": "github"
  },
  "version": "0.5.0"
}

This may be related to #113

@zanieb
Copy link
Author

zanieb commented Nov 8, 2024

I briefly started working on an updater patch, but first we need to patch the installer to fix the existing release.

Here's the work-in-progress on the updater: main...zanieb:axoupdater:zb/layout

I applied this patch to the installer locally to test:

@@ -1021,6 +1021,16 @@
     elif [ -n "$UNMANAGED_INSTALL" ]; then
         _force_install_dir="$UNMANAGED_INSTALL"
         _install_layout="flat"
+    fi
+
+    # Workaround for https://github.com/axodotdev/axoupdater/issues/210
+    # This was manually added to the installer post-release to patch the 0.4.30 -> 0.5.0 upgrade
+    if [ -n "${_force_install_dir:-}" ]; then
+        if [ "$_force_install_dir" = "${HOME:-}/.cargo" ] || [ "$_force_install_dir" = "${CARGO_HOME:-}" ]; then
+            _install_layout="cargo-home"
+        else
+            _install_layout="flat"
+        fi
     fi
 
     # Before actually consulting the configured install strategy, see

Then patched the 0.5.0 artifact and tested a self-update

❯ which uv
/Users/zb/.cargo/bin/uv
❯ uv --version
uv 0.4.30 (61ed2a236 2024-11-04)
❯ uv self update
info: Checking for updates...
success: Upgraded uv from v0.4.30 to v0.5.0! https://github.com/astral-sh/uv/releases/tag/0.5.0
❯ uv --version
uv 0.5.0 (8d665267c 2024-11-07)
❯ which uv
/Users/zb/.cargo/bin/uv

@zanieb
Copy link
Author

zanieb commented Nov 8, 2024

Next I'm testing a patch for the Windows installer

@@ -281,6 +281,17 @@
     $install_layout = "flat"
   }
   
+  # Workaround for https://github.com/axodotdev/axoupdater/issues/210
+  # This was manually added to the installer post-release to patch the 0.4.30 ->
+  # 0.5.0 upgrade
+  if (($force_install_dir)) {
+    if ($force_install_dir.Replace('\\', '\') -eq (Join-Path $HOME ".cargo") -or $force_install_dir -eq $env:CARGO_HOME) {
+      $install_layout = "cargo-home"
+    } else {
+      $install_layout = "flat"
+    }
+  }
+
   # The actual path we're going to install to
   $dest_dir = $null
   $dest_dir_lib = $null

Similarly, this worked locally so I updated the release artifact.

Notably force_install_dir.Replace('\\', '\') is really hacky, but I couldn't find a simple way to normalize the paths for comparison. Generally, this patch feels pretty sketchy.

@zanieb
Copy link
Author

zanieb commented Nov 8, 2024

I think we'll need every subsequent uv release to include these patches (or improved versions of them) since the updater for 0.4.30 will never read the install_layout, even if we fix this in a future updater version.

zanieb added a commit to astral-sh/uv that referenced this issue Nov 8, 2024
Gets us the upstream fix
(https://github.com/axodotdev/cargo-dist/pull/1538Z) for
axodotdev/axoupdater#210 so we don't need to
patch releases manually for self update to work.

Includes a few other changes, i.e., they validate checksums now.
@zanieb
Copy link
Author

zanieb commented Nov 8, 2024

Note these patches are not robust, see axodotdev/cargo-dist#1538 and its follow-ups. The next cargo-dist version will special case the cargo-home layout in the installer to fix this, but there still should be changes to resolve this in the long run.

konstin pushed a commit to astral-sh/uv that referenced this issue Nov 10, 2024
Gets us the upstream fix
(https://github.com/axodotdev/cargo-dist/pull/1538Z) for
axodotdev/axoupdater#210 so we don't need to
patch releases manually for self update to work.

Includes a few other changes, i.e., they validate checksums now.
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

No branches or pull requests

1 participant