ARGS: defaults is now always interpreted as a list

bits

@@ -224,6 +224,9 @@ do
         $BITSDIR/bitsBuild $@
 	    exit $?
         ;;
+    setenv)
+	break
+	;;
 	 *)
 	    shift
         ;;
@@ -278,7 +281,7 @@ do
       --help|help)
 	  printHelp; exit 0 ;;
       -c) 
-	  i+=1; COMMAND_IN_ENV=("$@");
+	  i=$((i+1)); COMMAND_IN_ENV=("${ARGV[@]:${i}}");
 	  break ;;
       *)
 	  ARGS+=(${ARGV[$i]});;

bits_helpers/args.py

@@ -413,10 +413,7 @@ def finaliseArgs(args, parser):
     return args
 
   if hasattr(args, "defaults"):
-    if "::" in args.defaults:
-      args.defaults,args.xdefaults = args.defaults.split("::")
-    else:
-      args.xdefaults = None
+    args.defaults = args.defaults.split("::")
 
   # --architecture can be specified in both clean and build.
   if args.action in ["build", "clean"] and not args.architecture:

bits_helpers/build.py

@@ -490,7 +490,7 @@ def doBuild(args, parser):
   if branch_stream == branch_basename:
     branch_stream = ""
 
-  defaultsReader = lambda : readDefaults(args.configDir, args.defaults, parser.error, args.architecture, args.xdefaults)
+  defaultsReader = lambda : readDefaults(args.configDir, args.defaults, parser.error, args.architecture)
   (err, overrides, taps) = parseDefaults(args.disable,
                                         defaultsReader, debug)
   dieOnError(err, err)
@@ -876,7 +876,7 @@ def performPreferCheckWithTempDir(pkg, cmd):
     if possibleDevelPrefix:
       spec["build_family"] = "%s-%s" % (possibleDevelPrefix, args.defaults)
     else:
-      spec["build_family"] = args.defaults
+      spec["build_family"] = "_".join(args.defaults)
     if spec["package"] == mainPackage:
       mainBuildFamily = spec["build_family"]
 
@@ -1176,6 +1176,7 @@ def performPreferCheckWithTempDir(pkg, cmd):
                       if "reference" in spec else ""),
       )
     else:
+      buildEnvironment = {key: (val if isinstance(val, str) else "_".join(val)) for key, val in buildEnvironment}
       os.environ.update(buildEnvironment)
       build_command = "%s -e -x %s/build.sh 2>&1" % (BASH, quote(scriptDir))
 
@@ -1315,14 +1316,12 @@ def performPreferCheckWithTempDir(pkg, cmd):
       syncHelper.upload_symlinks_and_tarball(spec)
 
   if not args.onlyDeps:
-      banner("Build of %s successfully completed on `%s'.\n"
+      banner(f"Build of {mainPackage} successfully completed on `{socket.gethostname()}'.\n"
              "Your software installation is at:"
-             "\n\n  %s\n\n"
+             f"\n\n  {abspath(join(args.workDir, args.architecture))}\n\n"
              "You can use this package by loading the environment:"
-             "\n\n  bits enter %s/latest-%s",
-             mainPackage, socket.gethostname(),
-             abspath(join(args.workDir, args.architecture)),
-             mainPackage, mainBuildFamily)
+             f"\n\n  bits enter {mainPackage}/latest-{mainBuildFamily}",
+             )
   else:
       banner("Successfully built dependencies for package %s on `%s'.\n",
              mainPackage, socket.gethostname()

bits_helpers/deps.py

@@ -14,7 +14,7 @@ def doDeps(args, parser):
 
   # Resolve all the package parsing boilerplate
   specs = {}
-  defaultsReader = lambda: readDefaults(args.configDir, args.defaults, parser.error, args.architecture, args.xdefaults)
+  defaultsReader = lambda: readDefaults(args.configDir, args.defaults, parser.error, args.architecture)
   (err, overrides, taps) = parseDefaults(args.disable, defaultsReader, debug)
 
   extra_env = {"BITS_CONFIG_DIR": "/alidist" if args.docker else path.abspath(args.configDir)}

bits_helpers/doctor.py

@@ -134,7 +134,7 @@ def doDoctor(args, parser):
   systemInfo()
 
   specs = {}
-  defaultsReader = lambda : readDefaults(args.configDir, args.defaults, parser.error, args.architecture, args.xdefaults)
+  defaultsReader = lambda : readDefaults(args.configDir, args.defaults, parser.error, args.architecture)
   (err, overrides, taps) = parseDefaults(args.disable, defaultsReader, info)
   if err:
     error("%s", err)

bits_helpers/init.py

@@ -45,7 +45,7 @@ def doInit(args):
   # Use standard functions supporting overrides and taps. Ignore all disables
   # and system packages as they are irrelevant in this context
   specs = {}
-  defaultsReader = lambda: readDefaults(args.configDir, args.defaults, lambda msg: error("%s", msg), args.architecture, args.xdefaults)
+  defaultsReader = lambda: readDefaults(args.configDir, args.defaults, lambda msg: error("%s", msg), args.architecture)
   (err, overrides, taps) = parseDefaults([], defaultsReader, debug)
   (_,_,_,validDefaults) = getPackageList(packages=[ p["name"] for p in pkgs ],
                                          specs=specs,

bits_helpers/utilities.py

@@ -149,7 +149,7 @@ def resolve_spec_data(spec, data, defaults, branch_basename="", branch_stream=""
 
   with the calculated content.
   """
-  defaults_upper = defaults != "release" and "_" + defaults.upper().replace("-", "_") or ""
+  defaults_upper = "" if defaults == ['release'] else "_".join(d.upper() for d in defaults)
   commit_hash = spec.get("commit_hash", "hash_unknown")
   tag = str(spec.get("tag", "tag_unknown"))
   package = spec.get("package")
@@ -333,51 +333,43 @@ def disabledByArchitectureDefaults(arch, defaults, requires):
     elif not re.match(matcher, arch):
       yield require
 
-
-def merge_ordered_dicts(dict1, dict2):
+def merge_dicts(dict1, dict2):
   """
-  Merge two ordered dictionaries where dict2's keys overwrite dict1's keys.
-  Preserves order with dict1's keys first, followed by dict2's new keys.
+  Merge two ordered dictionaries where dict2's keys updates dict1's keys recursively.
   """
-  merged = OrderedDict()
   # Add all keys from dict1 first
-  for key, value in dict1.items():
-    merged[key] = value
-
+  merged = dict1.copy()
   # Overwrite with dict2's values and add new keys
   for key, value in dict2.items():
-    if key in merged and isinstance(merged[key], OrderedDict) and isinstance(value, OrderedDict):
+    if key not in merged:
+      merged[key] = value
+      continue
+    elif isinstance(merged[key], dict) and isinstance(value, dict):
       # Recursively merge nested ordered dictionaries
-      merged[key] = merge_ordered_dicts(merged[key], value)
+      merged[key] = merge_dicts(merged[key], value)
+    elif isinstance(merged[key], list) and isinstance(value, list):
+      # merge lists, such as for "disabled"
+      merged[key].extend(value)
     else:
       # Overwrite existing key or add new key
       merged[key] = value
   return merged
-
-
-def readDefaults(configDir, defaults, error, architecture, xdefaults):
-
-  defaultsFilename = resolveDefaultsFilename(defaults,configDir)
-  err, defaultsMeta, defaultsBody = parseRecipe(getRecipeReader(defaultsFilename))
-  if err:
-    error(err)
-    sys.exit(1)
-
-  for x in ["overrides"]:
-    defaultsMeta[x] = asDict(defaultsMeta.get(x, OrderedDict()))
-
-  debug("Defaults: %s ",json.dumps(defaultsMeta,indent = 4))
-
-  if xdefaults is not None:
+
+def readDefaults(configDir, defaults, error, architecture):
+  defaultsMeta = {}
+  defaultsBody = ""
+
+  for xdefaults in defaults:
     xDefaults = resolveDefaultsFilename(xdefaults,configDir)
     xMeta = {}
-    xBody = ""
     if exists(xDefaults):
       err, xMeta, xBody = parseRecipe(getRecipeReader(xDefaults))
+      if xBody.strip() != "":
+        defaultsBody += "\n" + xBody.strip()
       if err:
         error(err)
         sys.exit(1)
-      defaultsMeta = merge_ordered_dicts(defaultsMeta, xMeta)
+      defaultsMeta = merge_dicts(defaultsMeta, xMeta)
 
   archDefaults = "%s/defaults-%s.sh" % (configDir, architecture)
   archMeta = {}
@@ -558,26 +550,6 @@ def asDict(overrides_array):
     debug("asDict (result): %s ",json.dumps(result))
     return result
 
-def merge_dicts(dict1, dict2):
-  """
-  Helper function to merge two dictionaries recursively.
-  dict2 values overwrite dict1 values.
-  """
-  merged = OrderedDict(dict1)
-
-  for key, value in dict2.items():
-    if (key in merged and 
-      isinstance(merged[key], dict) and 
-      isinstance(value, dict)):
-      # Recursively merge nested dictionaries
-      merged[key] = merge_dicts(merged[key], value)
-    else:
-      # Overwrite or add new key-value pair
-      merged[key] = value
-
-  return merged
-
-
 # (Almost pure part of the defaults parsing)
 # Override defaultsGetter for unit tests.
 def parseDefaults(disable, defaultsGetter, log):
@@ -674,10 +646,18 @@ def getPackageList(packages, specs, configDir, preferSystem, noSystem,
   packages = packages[:]
   generatedPackages = getGeneratedPackages(configDir)
   validDefaults = []  # empty list: all OK; None: no valid default; non-empty list: list of valid ones
-
   while packages:
     p = packages.pop(0)
-    if p in specs or (p == "defaults-release" and ("defaults-" + defaults) in specs):
+    if p in specs:
+      continue
+    skip = False
+    for d in defaults:
+      if p == "defaults-release" and ("defaults-" + d) in specs:
+        skip = True
+        break
+      else:
+        pkg_filename = ("defaults-" + d) if p == "defaults-release" else p.lower()
+    if skip:
       continue
 
     # We rewrite all defaults to "defaults-release", so load the correct
@@ -687,8 +667,6 @@ def getPackageList(packages, specs, configDir, preferSystem, noSystem,
     # they will end up with the same hash. The defaults must be called
     # "defaults-release" for this to work, since the defaults are a dependency
     # and all dependencies' names go into a package's hash.
-    pkg_filename = ("defaults-" + defaults) if p == "defaults-release" else p.lower()
-
     filename,pkgdir = resolveFilename(taps, pkg_filename, configDir, generatedPackages)
 
     dieOnError(not filename, "Package %s not found in %s" % (p, configDir))

tests/test_args.py

@@ -58,10 +58,10 @@ class FakeExit(Exception):
   ((), "build zlib -a slc7_x86-64 --docker-extra-args=--foo"                           , [("docker", True), ("dockerImage", "registry.cern.ch/alisw/slc7-builder"), ("docker_extra_args", ["--foo", "--network=host"])]),
   ((), "build zlib --devel-prefix -a slc7_x86-64 --docker"                             , [("docker", True), ("dockerImage", "registry.cern.ch/alisw/slc7-builder"), ("develPrefix", "%s-slc7_x86-64" % os.path.basename(os.getcwd()))]),
   ((), "build zlib --devel-prefix -a slc7_x86-64 --docker-image someimage"             , [("docker", True), ("dockerImage", "someimage"), ("develPrefix", "%s-slc7_x86-64" % os.path.basename(os.getcwd()))]),
-  ((), "--debug build --force-unknown-architecture --defaults o2 O2"                   , [("debug", True), ("action",  "build"), ("defaults", "o2"), ("pkgname", ["O2"])]),
-  ((), "build --force-unknown-architecture --debug --defaults o2 O2"                   , [("debug", True), ("action",  "build"), ("force_rebuild", []), ("defaults", "o2"), ("pkgname", ["O2"])]),
-  ((), "build --force-unknown-architecture --force-rebuild O2 --force-rebuild O2Physics --defaults o2 O2Physics", [("action", "build"), ("force_rebuild", ["O2", "O2Physics"]), ("defaults", "o2"), ("pkgname", ["O2Physics"])]),
-  ((), "build --force-unknown-architecture --force-rebuild O2,O2Physics --defaults o2 O2Physics", [("action", "build"), ("force_rebuild", ["O2", "O2Physics"]), ("defaults", "o2"), ("pkgname", ["O2Physics"])]),
+  ((), "--debug build --force-unknown-architecture --defaults o2 O2"                   , [("debug", True), ("action",  "build"), ("defaults", ["o2"]), ("pkgname", ["O2"])]),
+  ((), "build --force-unknown-architecture --debug --defaults o2 O2"                   , [("debug", True), ("action",  "build"), ("force_rebuild", []), ("defaults", ["o2"]), ("pkgname", ["O2"])]),
+  ((), "build --force-unknown-architecture --force-rebuild O2 --force-rebuild O2Physics --defaults o2 O2Physics", [("action", "build"), ("force_rebuild", ["O2", "O2Physics"]), ("defaults", ["o2"]), ("pkgname", ["O2Physics"])]),
+  ((), "build --force-unknown-architecture --force-rebuild O2,O2Physics --defaults o2 O2Physics", [("action", "build"), ("force_rebuild", ["O2", "O2Physics"]), ("defaults", ["o2"]), ("pkgname", ["O2Physics"])]),
   ((), "init -z test zlib"                                                             , [("configDir", "test/alidist")]),
   ((), "build --force-unknown-architecture -z test zlib"                               , [("configDir", "alidist")]),
   # ((), "analytics off"                                                                 , [("state", "off")]),

tests/test_build.py

@@ -275,7 +275,7 @@ def test_coverDoBuild(self, mock_debug, mock_listdir, mock_warning, mock_git_git
             configDir="/alidist",
             disable=[],
             force_rebuild=[],
-            defaults="release",
+            defaults=["release"],
             jobs=2,
             annotate={},
             preferSystem=[],
@@ -291,7 +291,6 @@ def test_coverDoBuild(self, mock_debug, mock_listdir, mock_warning, mock_git_git
             forceTracked=False,
             plugin="legacy",
             makeflow=False,
-            xdefaults=None
         )
 
         def mkcall(args):

tests/test_deepmergedefaults.py

@@ -0,0 +1,77 @@
+import unittest
+import yaml
+import bits_helpers.utilities
+from bits_helpers.utilities import merge_dicts
+
+class DeepMergeTest(unittest.TestCase):
+    # Test overwriting existing top-level keys from dict1 with top-level keys from dict2.
+    # Test adding new top level keys from dict2.
+    def test_flat_merge(self):
+        d1 = ({"a": 1, "b": 2})
+        d2 = ({"b": 3, "c": 4})
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"a": 1, "b": 3, "c": 4})
+
+    # Test nested merge of dicts within dicts
+    def test_nested_merge(self):
+        d1 = {
+            "a": 1,
+            "b": {"x": 1, "y": 2},
+        }
+        d2 = {
+            "b": {"y": 3, "z": 4},
+            "c": 5,
+        }
+        result = merge_dicts(d1, d2)
+        expected = {"a": 1, "b": {"x": 1, "y": 3, "z": 4}, "c": 5}
+        self.assertEqual(result, expected)
+
+    # Test merging of lists
+    def test_list_merge(self):
+        d1 = {"disabled": ["pkg1", "pkg2"]}
+        d2 = {"disabled": ["pkg3"]}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"disabled": ["pkg1", "pkg2", "pkg3"]})
+
+    # Test non-recursive overwrite on type mismatch between dict and non-dict
+    def test_overwrite_non_dict(self):
+        d1 = {"a": {"nested": 1}}
+        d2 = {"a": 42}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"a": 42})
+
+    # Test merging of non-overlapping dictionaries
+    def test_new_key_added(self):
+        d1 = {"a": 1}
+        d2 = {"b": 2}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"a": 1, "b": 2})
+
+    # Test merging of empty dictionaries
+    def test_empty_dicts(self):
+        d1 = {}
+        d2 = {}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {})
+
+    # Test merging a non-empty first with empty second dict
+    def test_merge_with_empty_second(self):
+        d1 = {"a": 1}
+        d2 = {}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"a": 1})
+
+    # Test merging a empty first with non-empty second dict
+    def test_merge_with_empty_first(self):
+        d1 = {}
+        d2 = {"b": 2}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"b": 2})
+
+    # Test type mismatch between list and non-list
+    def test_list_non_list_conflict(self):
+        """When one side is list and the other is not, prefer dict2."""
+        d1 = {"disabled": ["pkg1", "pkg2"]}
+        d2 = {"disabled": "notalist"}
+        result = merge_dicts(d1, d2)
+        self.assertEqual(result, {"disabled": "notalist"})