From ffe3e043f216c31b880e0fe2039b0a400a3bf218 Mon Sep 17 00:00:00 2001
From: Nick Anderson <nick@cmdln.org>
Date: Wed, 3 Jun 2026 11:59:36 -0500
Subject: [PATCH] Fixed cfbs update NAME@VERSION

The @VERSION was parsed but discarded: only the module name was passed to
get_module_object(), so update always resolved the latest index entry and
wrote that to cfbs.json while reporting the requested version. Now the
pinned version is resolved from the version index, unknown versions are
skipped with a warning.

Ticket: ENT-14170
Changelog: Made cfbs update honor a pinned module version (NAME@VERSION)
---
 cfbs/commands.py                              |  9 +++-
 cfbs/index.py                                 |  5 ++
 ...050_update_masterfiles_specific_version.sh | 51 +++++++++++++++++++
 tests/shell/all.sh                            |  1 +
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 tests/shell/050_update_masterfiles_specific_version.sh

diff --git a/cfbs/commands.py b/cfbs/commands.py
index d6505236..18baf830 100644
--- a/cfbs/commands.py
+++ b/cfbs/commands.py
@@ -712,7 +712,14 @@ def update_command(to_update):
                 log.debug("Module '%s' has no version attribute." % old_module["name"])
                 continue
 
-            index_info = index.get_module_object(update.name)
+            if update.version and not index.exists(update):
+                log.warning(
+                    "Module '%s' version '%s' is not present in the index,"
+                    " cannot update it." % (old_module["name"], update.version)
+                )
+                continue
+
+            index_info = index.get_module_object(update)
             if not index_info:
                 log.warning(
                     "Module '%s' not present in the index, cannot update it."
diff --git a/cfbs/index.py b/cfbs/index.py
index 5c4a2960..41f0e8ca 100644
--- a/cfbs/index.py
+++ b/cfbs/index.py
@@ -245,6 +245,11 @@ def get_module_object(
                 }
                 object.update(specifics)
                 object["version"] = version
+                # The version index stores an empty "subdirectory" for
+                # modules without one; an empty subdirectory is invalid, so
+                # drop it (matches how 'cfbs add' cleans the module up).
+                if object.get("subdirectory") == "":
+                    del object["subdirectory"]
 
         assert object is not None
         module.update(object)
diff --git a/tests/shell/050_update_masterfiles_specific_version.sh b/tests/shell/050_update_masterfiles_specific_version.sh
new file mode 100644
index 00000000..d9254417
--- /dev/null
+++ b/tests/shell/050_update_masterfiles_specific_version.sh
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+set -e
+set -x
+cd tests/
+mkdir -p ./tmp/
+cd ./tmp/
+touch cfbs.json && rm cfbs.json
+rm -rf .git
+
+# Inspect the masterfiles module specifically, rather than grepping the whole
+# file (which could match a field belonging to a different module).
+mf() { jq -r --arg k "$1" '.build[] | select(.name == "masterfiles") | .[$k]' cfbs.json; }
+mf_has() { jq -r --arg k "$1" '.build[] | select(.name == "masterfiles") | has($k)' cfbs.json; }
+
+# Start on an old version of masterfiles.
+cfbs --non-interactive init --masterfiles=3.24.1
+[ "$(mf version)" = "3.24.1" ]
+[ "$(mf commit)" = "1171e2e50a229d78e2fdd4357a5d07ecc19bdbf4" ]
+
+# Update to a specific intermediate version (not the latest). This must pin to
+# exactly 3.24.4, not jump to the newest release in the index.
+cfbs --non-interactive update masterfiles@3.24.4
+[ "$(mf version)" = "3.24.4" ]
+[ "$(mf commit)" = "ed4628805e352fd68d7d72664c859df5a4bb0715" ]
+# Why we assert on "subdirectory" here: pinning an update to a version now routes
+# through index.get_module_object() *with that version*, which reads the
+# per-version index. That index stores an empty "subdirectory" ("") for modules
+# that don't have one, and an empty subdirectory is invalid and breaks validation
+# and build. So get_module_object() now has to drop it (matching how 'cfbs add'
+# cleans the module up). This asserts it does not leak into cfbs.json, and the
+# following 'cfbs validate' confirms the resulting module is actually valid.
+[ "$(mf_has subdirectory)" = "false" ]
+cfbs validate
+
+# Asking again for the same version is a no-op.
+cfbs --non-interactive update masterfiles@3.24.4
+[ "$(mf version)" = "3.24.4" ]
+
+# Asking for an older version than the current one must not downgrade.
+cfbs --non-interactive update masterfiles@3.24.1
+[ "$(mf version)" = "3.24.4" ]
+
+# A plain update (no @version) still moves to a strictly newer version than the
+# pin. Checking "!= 3.24.4" alone would also pass on a downgrade, so verify that
+# 3.24.4 sorts before the new version (i.e. the new version is the greater one).
+cfbs --non-interactive update masterfiles
+new_ver="$(mf version)"
+[ "$new_ver" != "3.24.4" ]
+[ "$(printf '%s\n%s\n' "3.24.4" "$new_ver" | sort -V | tail -1)" = "$new_ver" ]
+
+cfbs build
diff --git a/tests/shell/all.sh b/tests/shell/all.sh
index 11400bbb..71fc5895 100644
--- a/tests/shell/all.sh
+++ b/tests/shell/all.sh
@@ -93,6 +93,7 @@ run_test tests/shell/046_update_from_url_branch.sh
 run_test tests/shell/047_absolute_path_modules.sh
 run_test tests/shell/048_remove_with_dependencies.sh
 run_test tests/shell/049_remove_with_circular_dependencies.sh
+run_test tests/shell/050_update_masterfiles_specific_version.sh
 
 # Summary
 _suite_end=$(date +%s)