axodotdev · mistydemeo · Apr 17, 2024 · Apr 17, 2024 · Apr 17, 2024
diff --git a/cargo-dist/templates/installer/installer.ps1.j2 b/cargo-dist/templates/installer/installer.ps1.j2
@@ -120,6 +120,18 @@ function Get-TargetTriple() {
   }
 }
 
+# Provides a way to migrate from an old "flat" layout to the new
+# FHS-style organized layout.
+function MigrateInstallDir($old_root, $new_root, $bin_paths) {
+  foreach ($bin_name in $bin_paths) {
+    $bin_path = "$old_root\$bin_name"
+      if (Test-Path -Path "$bin_path") {
+        Copy-Item "$bin_path" -Destination "$new_root"
+        Write-Verbose "Migrated $bin_path to $new_root\$bin_name"
+      }
+  }
+}
+
 function Download($download_url, $platforms) {
   $arch = Get-TargetTriple
 
@@ -213,28 +225,36 @@ function Invoke-Installer($bin_paths, $platforms) {
   }
 {% elif install_path.kind == "HomeSubdir" %}
   # Install to this subdir of the user's home dir
-  $dest_dir = if (($base_dir = $HOME)) {
+  $root = if (($base_dir = $HOME)) {
     Join-Path $base_dir "{{ install_path.subdir }}"
   } else {
     throw "ERROR: could not find your HOME dir to install binaries to"
   }
 
   # If we need to override the above
   if (($env:CARGO_DIST_FORCE_INSTALL_DIR)) {
-    $dest_dir = $env:CARGO_DIST_FORCE_INSTALL_DIR
+    $root = $env:CARGO_DIST_FORCE_INSTALL_DIR
   }
+
+  $dest_dir = Join-Path $root "bin"
+
+  MigrateInstallDir -old_root $root -new_root $dest_dir -bin_paths $bin_paths
 {% elif install_path.kind == "EnvSubdir" %}
   # Install to this subdir of the user's {{ install_path.env_key }} dir
-  $dest_dir = if (($base_dir = $env:{{ install_path.env_key }})) {
+  $root = if (($base_dir = $env:{{ install_path.env_key }})) {
     Join-Path $base_dir "{{ install_path.subdir }}"
   } else {
     throw "ERROR: could not find your {{ install_path.env_key }} dir to install binaries to"
   }
 
   # If we need to override the above
   if (($env:CARGO_DIST_FORCE_INSTALL_DIR)) {
-    $dest_dir = $env:CARGO_DIST_FORCE_INSTALL_DIR
+    $root = $env:CARGO_DIST_FORCE_INSTALL_DIR
   }
+
+  $dest_dir = Join-Path $root "bin"
+
+  MigrateInstallDir -old_root $root -new_root $dest_dir -bin_paths $bin_paths
 {% else %}
   {{ error("unimplemented install_path format: " ~ install_path.kind) }}
 {% endif %}

diff --git a/cargo-dist/templates/installer/installer.sh.j2 b/cargo-dist/templates/installer/installer.sh.j2
@@ -241,9 +241,41 @@ download_binary_and_run_installer() {
     return "$_retval"
 }
 
+# Provides a way to migrate from an old "flat" layout to the new
+# FHS-style organized layout.
+migrate_install_dir() {
+    local _old_root="$1"
+    local _new_root="$2"
+    local _bins="$3"
+
+    ensure mkdir -p "$_new_root"
+    # Check for a previous installation that's in the root
+    # instead of the root plus /bin; this may be left over
+    # from before we migrated to always use FHS-style layouts.
+    for _bin_name in $_bins; do
+        local _old_path="$_old_root/$_bin_name"
+        local _new_path="$_new_root/$_bin_name"
+        # We move this into the new path expecting them
+        # to be overwritten later. Doing that, instead of
+        # just deleting them now, guards against the
+        # possibility that the install fails: if that were
+        # to happen, we'd have deleted them without
+        # producing new copies, leaving the user without
+        # any binaries left at all.
+        if [ -f "$_old_path" ]; then
+            ensure mv "$_old_path" "$_new_path"
+            say_verbose "Migrated $_old_path to $_new_path"
+        fi
+    done
+}
+
 # See discussion of late-bound vs early-bound for why we use single-quotes with env vars
 # shellcheck disable=SC2016
 install() {
+    # Assign parameters immediately, before doing anything else
+    local _src_dir="$1"
+    local _bins="$2"
+
     # This code needs to both compute certain paths for itself to write to, and
     # also write them to shell/rc files so that they can look them up to e.g.
     # add them to PATH. This requires an active distinction between paths
@@ -336,38 +368,48 @@ install() {
     # Install to this subdir of the user's home dir
     # In this case we want to be late-bound, as $HOME is reliable/nice.
     if [ -n "${HOME:-}" ]; then
-        _install_dir="$HOME/{{ install_path.subdir }}"
+        _install_root="$HOME/{{ install_path.subdir }}"
+        _install_dir="$HOME/{{ install_path.subdir }}/bin"
         _env_script_path="$HOME/{{ install_path.subdir }}/env"
-        _install_dir_expr='$HOME/{{ install_path.subdir }}'
+        _install_dir_expr='$HOME/{{ install_path.subdir }}/bin'
         _env_script_path_expr='$HOME/{{ install_path.subdir }}/env'
 
         if [ -n "${CARGO_DIST_FORCE_INSTALL_DIR:-}" ]; then
-            _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR"
+            _install_root="$CARGO_DIST_FORCE_INSTALL_DIR"
+            _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR/bin"
             _env_script_path="$CARGO_DIST_FORCE_INSTALL_DIR/env"
             # For these two, if the new value contains $HOME, make
             # sure to `sed` the $HOME expression back into the expr
             _install_dir_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR" | sed "s,$HOME,\$HOME,")"
             _env_script_path_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR/env" | sed "s,$HOME,\$HOME,")"
         fi
+
+        # Migrate from the old flat directory path to the new location
+        migrate_install_dir "$_install_root" "$_install_dir" "$_bins"
     else
         err "could not find your HOME dir to install binaries to"
     fi
 {% elif install_path.kind == "EnvSubdir" %}
     # Install to this subdir of the user's {{ install_path.env_key }} dir.
     # In this case we want to be early-bound, as the env-var can't be trusted longterm.
     if [ -n "{{ "${" }}{{ install_path.env_key }}:-}" ]; then
-        _install_dir="${{ install_path.env_key }}{% if install_path.subdir | length %}/{% endif %}{{ install_path.subdir }}"
+        _install_root="${{ install_path.env_key }}{% if install_path.subdir | length %}/{% endif %}{{ install_path.subdir }}"
+        _install_dir="$_install_root/bin"
         _env_script_path="${{ install_path.env_key }}{% if install_path.subdir | length %}/{% endif %}{{ install_path.subdir }}/env"
         _install_dir_expr="$_install_dir"
         _env_script_path_expr="$_env_script_path"
 
         # Override if necessary
         if [ -n "${CARGO_DIST_FORCE_INSTALL_DIR:-}" ]; then
-            _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR"
+            _install_root="$CARGO_DIST_FORCE_INSTALL_DIR"
+            _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR/bin"
             _env_script_path="$CARGO_DIST_FORCE_INSTALL_DIR/env"
             _install_dir_expr="$_install_dir"
             _env_script_path_expr="$_env_script_path"
         fi
+
+        # Migrate from the old flat directory path to the new location
+        migrate_install_dir "$_install_root" "$_install_dir" "$_bins"
     else
         err "could not find your {{ install_path.env_key }} dir to install binaries to"
     fi
@@ -382,8 +424,6 @@ install() {
     ensure mkdir -p "$_install_dir"
 
     # copy all the binaries to the install dir
-    local _src_dir="$1"
-    local _bins="$2"
     for _bin_name in $_bins; do
         local _bin="$_src_dir/$_bin_name"
         ensure mv "$_bin" "$_install_dir"

diff --git a/cargo-dist/tests/gallery/dist.rs b/cargo-dist/tests/gallery/dist.rs
@@ -679,6 +679,23 @@ impl DistResult {
             }
             std::fs::create_dir_all(&tempdir).unwrap();
 
+            let expected_bin_dir = Utf8PathBuf::from(expected_bin_dir);
+            let bin_dir = tempdir.join(&expected_bin_dir);
+            // Test generating fake old binaries within the older
+            // flat directory structure, allowing us to simulate
+            // performing a migration
+            if !expected_bin_dir
+                .components()
+                .any(|d| d.as_str() == ".cargo")
+            {
+                let root = bin_dir.parent().unwrap();
+                std::fs::create_dir_all(root).unwrap();
+                for bin_name in ctx.repo.bins {
+                    let old_path = root.join(bin_name);
+                    std::fs::File::create(old_path).unwrap();
+                }
+            }
+
             // Run the installer script with:
             //
             // HOME="{tempdir}"            (for install-path=~/... and install-path=CARGO_HOME)
@@ -700,16 +717,7 @@ impl DistResult {
                 tempdir.join(".zshrc"),
             ];
             let receipt_file = tempdir.join(format!(".config/{app_name}/{app_name}-receipt.json"));
-            let expected_bin_dir = Utf8PathBuf::from(expected_bin_dir);
-            let bin_dir = tempdir.join(&expected_bin_dir);
-            let env_dir = if expected_bin_dir
-                .components()
-                .any(|d| d.as_str() == ".cargo")
-            {
-                bin_dir.parent().unwrap()
-            } else {
-                &bin_dir
-            };
+            let env_dir = bin_dir.parent().unwrap();
             let env_script = env_dir.join("env");
 
             assert!(bin_dir.exists(), "bin dir wasn't created");

diff --git a/cargo-dist/tests/integration-tests.rs b/cargo-dist/tests/integration-tests.rs
@@ -1016,7 +1016,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/")?.snap();
+        results.check_all(ctx, ".axolotlsay/bin/")?.snap();
 
         Ok(())
     })
@@ -1043,7 +1043,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/bins")?.snap();
+        results.check_all(ctx, ".axolotlsay/bins/bin/")?.snap();
 
         Ok(())
     })
@@ -1070,7 +1070,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, "My Axolotlsay Documents/")?.snap();
+        results.check_all(ctx, "My Axolotlsay Documents/bin/")?.snap();
 
         Ok(())
     })
@@ -1096,7 +1096,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, "My Axolotlsay Documents/bin/")?.snap();
+        results.check_all(ctx, "My Axolotlsay Documents/bin/bin/")?.snap();
 
         Ok(())
     })
@@ -1123,7 +1123,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/")?.snap();
+        results.check_all(ctx, ".axolotlsay/bin/")?.snap();
 
         Ok(())
     })
@@ -1150,7 +1150,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/bin/")?.snap();
+        results.check_all(ctx, ".axolotlsay/bin/bin/")?.snap();
 
         Ok(())
     })
@@ -1177,7 +1177,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/My Axolotlsay Documents/")?.snap();
+        results.check_all(ctx, ".axolotlsay/My Axolotlsay Documents/bin/")?.snap();
 
         Ok(())
     })
@@ -1204,7 +1204,7 @@ windows-archive = ".tar.gz"
         ))?;
 
         let results = ctx.cargo_dist_build_and_plan(test_name)?;
-        results.check_all(ctx, ".axolotlsay/My Axolotlsay Documents/bin/")?.snap();
+        results.check_all(ctx, ".axolotlsay/My Axolotlsay Documents/bin/bin/")?.snap();
 
         Ok(())
     })

diff --git a/cargo-dist/tests/snapshots/akaikatana_basic.snap b/cargo-dist/tests/snapshots/akaikatana_basic.snap
@@ -246,9 +246,41 @@ download_binary_and_run_installer() {
     return "$_retval"
 }
 
+# Provides a way to migrate from an old "flat" layout to the new
+# FHS-style organized layout.
+migrate_install_dir() {
+    local _old_root="$1"
+    local _new_root="$2"
+    local _bins="$3"
+
+    ensure mkdir -p "$_new_root"
+    # Check for a previous installation that's in the root
+    # instead of the root plus /bin; this may be left over
+    # from before we migrated to always use FHS-style layouts.
+    for _bin_name in $_bins; do
+        local _old_path="$_old_root/$_bin_name"
+        local _new_path="$_new_root/$_bin_name"
+        # We move this into the new path expecting them
+        # to be overwritten later. Doing that, instead of
+        # just deleting them now, guards against the
+        # possibility that the install fails: if that were
+        # to happen, we'd have deleted them without
+        # producing new copies, leaving the user without
+        # any binaries left at all.
+        if [ -f "$_old_path" ]; then
+            ensure mv "$_old_path" "$_new_path"
+            say_verbose "Migrated $_old_path to $_new_path"
+        fi
+    done
+}
+
 # See discussion of late-bound vs early-bound for why we use single-quotes with env vars
 # shellcheck disable=SC2016
 install() {
+    # Assign parameters immediately, before doing anything else
+    local _src_dir="$1"
+    local _bins="$2"
+
     # This code needs to both compute certain paths for itself to write to, and
     # also write them to shell/rc files so that they can look them up to e.g.
     # add them to PATH. This requires an active distinction between paths
@@ -346,8 +378,6 @@ install() {
     ensure mkdir -p "$_install_dir"
 
     # copy all the binaries to the install dir
-    local _src_dir="$1"
-    local _bins="$2"
     for _bin_name in $_bins; do
         local _bin="$_src_dir/$_bin_name"
         ensure mv "$_bin" "$_install_dir"
@@ -998,6 +1028,18 @@ function Get-TargetTriple() {
   }
 }
 
+# Provides a way to migrate from an old "flat" layout to the new
+# FHS-style organized layout.
+function MigrateInstallDir($old_root, $new_root, $bin_paths) {
+  foreach ($bin_name in $bin_paths) {
+    $bin_path = "$old_root\$bin_name"
+      if (Test-Path -Path "$bin_path") {
+        Copy-Item "$bin_path" -Destination "$new_root"
+        Write-Verbose "Migrated $bin_path to $new_root\$bin_name"
+      }
+  }
+}
+
 function Download($download_url, $platforms) {
   $arch = Get-TargetTriple
 

diff --git a/cargo-dist/tests/snapshots/akaikatana_musl.snap b/cargo-dist/tests/snapshots/akaikatana_musl.snap
@@ -258,9 +258,41 @@ download_binary_and_run_installer() {
     return "$_retval"
 }
 
+# Provides a way to migrate from an old "flat" layout to the new
+# FHS-style organized layout.
+migrate_install_dir() {
+    local _old_root="$1"
+    local _new_root="$2"
+    local _bins="$3"
+
+    ensure mkdir -p "$_new_root"
+    # Check for a previous installation that's in the root
+    # instead of the root plus /bin; this may be left over
+    # from before we migrated to always use FHS-style layouts.
+    for _bin_name in $_bins; do
+        local _old_path="$_old_root/$_bin_name"
+        local _new_path="$_new_root/$_bin_name"
+        # We move this into the new path expecting them
+        # to be overwritten later. Doing that, instead of
+        # just deleting them now, guards against the
+        # possibility that the install fails: if that were
+        # to happen, we'd have deleted them without
+        # producing new copies, leaving the user without
+        # any binaries left at all.
+        if [ -f "$_old_path" ]; then
+            ensure mv "$_old_path" "$_new_path"
+            say_verbose "Migrated $_old_path to $_new_path"
+        fi
+    done
+}
+
 # See discussion of late-bound vs early-bound for why we use single-quotes with env vars
 # shellcheck disable=SC2016
 install() {
+    # Assign parameters immediately, before doing anything else
+    local _src_dir="$1"
+    local _bins="$2"
+
     # This code needs to both compute certain paths for itself to write to, and
     # also write them to shell/rc files so that they can look them up to e.g.
     # add them to PATH. This requires an active distinction between paths
@@ -358,8 +390,6 @@ install() {
     ensure mkdir -p "$_install_dir"
 
     # copy all the binaries to the install dir
-    local _src_dir="$1"
-    local _bins="$2"
     for _bin_name in $_bins; do
         local _bin="$_src_dir/$_bin_name"
         ensure mv "$_bin" "$_install_dir"