Fixes for #1188; custom_commands broken (#1332)

Co-authored-by: GideonBear <87426140+GideonBear@users.noreply.github.com>

.github/workflows/ci.yml

@@ -28,8 +28,8 @@ jobs:
           rustup component add rustfmt
           cargo fmt --all -- --check
 
-  step-enum-sorted:
+  custom-checks:
-    name: Step enum sorted
+    name: Custom checks
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
@@ -45,13 +45,6 @@ jobs:
           sort original.txt > sorted.txt
           diff original.txt sorted.txt
 
-  step-match-sorted:
-    name: Step match sorted
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout code
-        uses: actions/checkout@v5.0.0
-
       - name: Check if `Step::run()`'s match is sorted
         run: |
           FILE="src/step.rs"
@@ -59,8 +52,40 @@ jobs:
           sort original.txt > sorted.txt
           diff original.txt sorted.txt
 
+      - name: Check if `default_steps` contains every step
+        run: |
+          # Extract all variants from enum Step
+          all_variants=$(sed -n '/^pub enum Step {/,/^}/p' src/step.rs | grep -Po '^\s*\K[A-Z][A-Za-z0-9_]*' | sort)
+
+          # Extract variants used inside default_steps
+          used_variants=$(sed -n '/^pub(crate) fn default_steps()/,/^}/p' src/step.rs | \
+            grep -Po '\b[A-Z][A-Za-z0-9_]*\b' | \
+            grep -Fx -f <(echo "$all_variants") | \
+          sort)
+
+          # Check for missing variants
+          missing=$(comm -23 <(echo "$all_variants") <(echo "$used_variants"))
+          if [[ -z "$missing" ]]; then
+            echo "All variants are used."
+            else
+            echo "Missing variants:"
+            echo "$missing"
+            exit 1
+          fi
+
+          # Check for duplicates
+          duplicates=$(echo "$used_variants" | uniq -c | awk '$1 > 1 {print $2}')
+          if [[ -z "$duplicates" ]]; then
+            echo "No duplicates found."
+            else
+            echo "Duplicates found:"
+            echo "$duplicates"
+            # We allow duplicates, but lets keep this check for potential future usefulness
+            # exit 1
+          fi
+
   main:
-    needs: [ fmt, step-enum-sorted, step-match-sorted ]
+    needs: [ fmt, custom-checks ]
     name: ${{ matrix.target_name }} (check, clippy)
     runs-on: ${{ matrix.os }}
     strategy:

src/step.rs

@@ -269,9 +269,12 @@ impl Step {
             }
             Containers => runner.execute(*self, "Containers", || containers::run_containers(ctx))?,
             CustomCommands => {
-                if let Some(commands) = ctx.config().pre_commands() {
+                if let Some(commands) = ctx.config().commands() {
-                    for (name, command) in commands {
+                    for (name, command) in commands
-                        generic::run_custom_command(name, command, ctx)?;
+                        .iter()
+                        .filter(|(n, _)| ctx.config().should_run_custom_command(n))
+                    {
+                        runner.execute(*self, name.clone(), || generic::run_custom_command(name, command, ctx))?;
                     }
                 }
             }
@@ -477,7 +480,7 @@ impl Step {
             PlatformioCore => runner.execute(*self, "PlatformIO Core", || generic::run_platform_io(ctx))?,
             Pnpm => runner.execute(*self, "pnpm", || node::run_pnpm_upgrade(ctx))?,
             Poetry => runner.execute(*self, "Poetry", || generic::run_poetry(ctx))?,
-            Powershell => runner.execute(Powershell, "Powershell Modules Update", || generic::run_powershell(ctx))?,
+            Powershell => runner.execute(*self, "Powershell Modules Update", || generic::run_powershell(ctx))?,
             Protonup =>
             {
                 #[cfg(target_os = "linux")]
@@ -500,7 +503,7 @@ impl Step {
                         .iter()
                         .filter(|t| ctx.config().should_execute_remote(hostname(), t))
                     {
-                        runner.execute(Remotes, format!("Remote ({remote_topgrade})"), || {
+                        runner.execute(*self, format!("Remote ({remote_topgrade})"), || {
                             crate::ssh::ssh_step(ctx, remote_topgrade)
                         })?;
                     }
@@ -577,7 +580,7 @@ impl Step {
 
                     match ctx.distribution() {
                         Ok(distribution) => {
-                            runner.execute(System, "System update", || distribution.upgrade(ctx))?;
+                            runner.execute(*self, "System update", || distribution.upgrade(ctx))?;
                         }
                         Err(e) => {
                             println!("{}", t!("Error detecting current distribution: {error}", error = e));
@@ -615,13 +618,13 @@ impl Step {
                 if ctx.config().should_run(Vagrant) {
                     if let Ok(boxes) = vagrant::collect_boxes(ctx) {
                         for vagrant_box in boxes {
-                            runner.execute(Vagrant, format!("Vagrant ({})", vagrant_box.smart_name()), || {
+                            runner.execute(*self, format!("Vagrant ({})", vagrant_box.smart_name()), || {
                                 vagrant::topgrade_vagrant_box(ctx, &vagrant_box)
                             })?;
                         }
                     }
                 }
-                runner.execute(Vagrant, "Vagrant boxes", || vagrant::upgrade_vagrant_boxes(ctx))?;
+                runner.execute(*self, "Vagrant boxes", || vagrant::upgrade_vagrant_boxes(ctx))?;
             }
             Vcpkg => runner.execute(*self, "vcpkg", || generic::run_vcpkg_update(ctx))?,
             Vim => {
@@ -692,26 +695,31 @@ pub(crate) fn default_steps() -> Vec<Step> {
     // initial and shrink
     let mut steps = Vec::with_capacity(Step::COUNT);
 
+    // Not combined with other generic steps to preserve the order as it was in main.rs originally,
+    // but this can be changed in the future.
+    steps.push(Remotes);
+
     #[cfg(windows)]
-    steps.extend_from_slice(&[Wsl, WslUpdate, Chocolatey, Scoop, Winget]);
+    steps.extend_from_slice(&[Wsl, WslUpdate, Chocolatey, Scoop, Winget, System, MicrosoftStore]);
 
     #[cfg(target_os = "macos")]
-    steps.extend_from_slice(&[BrewFormula, BrewCask, Macports, Xcodes, Sparkle, Mas]);
+    steps.extend_from_slice(&[BrewFormula, BrewCask, Macports, Xcodes, Sparkle, Mas, System]);
 
     #[cfg(target_os = "dragonfly")]
     steps.extend_from_slice(&[Pkg, Audit]);
 
-    #[cfg(any(target_os = "freebsd", target_os = "openbsd"))]
+    #[cfg(target_os = "freebsd")]
+    steps.extend_from_slice(&[Pkg, System, Audit]);
+
+    #[cfg(target_os = "openbsd")]
+    steps.extend_from_slice(&[Pkg, System]);
+
+    #[cfg(target_os = "android")]
     steps.push(Pkg);
 
-    #[cfg(not(any(target_os = "dragonfly", target_os = "android")))]
-    steps.push(System);
-
-    #[cfg(windows)]
-    steps.push(MicrosoftStore);
-
     #[cfg(target_os = "linux")]
     steps.extend_from_slice(&[
+        System,
         ConfigUpdate,
         AM,
         AppMan,
@@ -735,13 +743,11 @@ pub(crate) fn default_steps() -> Vec<Step> {
         Pkgfile,
     ]);
 
-    #[cfg(target_os = "freebsd")]
-    steps.push(Audit);
-
     #[cfg(unix)]
     steps.extend_from_slice(&[
         Yadm,
         Nix,
+        NixHelper,
         Guix,
         HomeManager,
         Asdf,