Merge pull request #313 from nickanderson/ENT-14170/master

ENT-14170: Fixed updating to a pinned module version

Author: nickanderson

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."

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)

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

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)