Merge pull request #82714 from porglezomp-misc/update-checkout-additional-scheme

porglezomp · web-flow · commit f7ccb5298fb5 · 2025-07-07T17:31:46.000-07:00
update-checkout: Support specifying --config multiple times
diff --git a/utils/update_checkout/tests/scheme_mock.py b/utils/update_checkout/tests/scheme_mock.py
@@ -62,6 +62,21 @@
     }
 }
 
+MOCK_ADDITIONAL_SCHEME = {
+    'branch-schemes': {
+        'extra': {
+            'aliases': ['extra'],
+            'repos': {
+                # Spell this differently just to make it distinguishable in
+                # test output, even though we only have one branch.
+                # TODO: Support multiple test branches in the repo instead.
+                'repo1': 'refs/heads/main',
+                'repo2': 'refs/heads/main',
+            }
+        }
+    }
+}
+
 
 class CallQuietlyException(Exception):
     def __init__(self, command, returncode, output):
@@ -78,7 +93,7 @@ def __str__(self):
 def call_quietly(*args, **kwargs):
     kwargs['stderr'] = subprocess.STDOUT
     try:
-        subprocess.check_output(*args, **kwargs)
+        return subprocess.check_output(*args, **kwargs)
     except subprocess.CalledProcessError as e:
         raise CallQuietlyException(command=e.cmd, returncode=e.returncode,
                                    output=e.stdout) from e
@@ -97,6 +112,10 @@ def get_config_path(base_dir):
     return os.path.join(base_dir, 'test-config.json')
 
 
+def get_additional_config_path(base_dir):
+    return os.path.join(base_dir, 'test-additional-config.json')
+
+
 def setup_mock_remote(base_dir, base_config):
     create_dir(base_dir)
 
@@ -142,6 +161,9 @@ def setup_mock_remote(base_dir, base_config):
     with open(get_config_path(base_dir), 'w') as f:
         json.dump(base_config, f)
 
+    with open(get_additional_config_path(base_dir), 'w') as f:
+        json.dump(MOCK_ADDITIONAL_SCHEME, f)
+
     return (LOCAL_PATH, REMOTE_PATH)
 
 
@@ -164,6 +186,7 @@ def __init__(self, *args, **kwargs):
             raise RuntimeError('Misconfigured test suite! Environment '
                                'variable %s must be set!' % BASEDIR_ENV_VAR)
         self.config_path = get_config_path(self.workspace)
+        self.additional_config_path = get_additional_config_path(self.workspace)
         self.update_checkout_path = UPDATE_CHECKOUT_PATH
         if not os.access(self.update_checkout_path, os.X_OK):
             raise RuntimeError('Error! Could not find executable '
@@ -182,7 +205,7 @@ def tearDown(self):
 
     def call(self, *args, **kwargs):
         kwargs['cwd'] = self.source_root
-        call_quietly(*args, **kwargs)
+        return call_quietly(*args, **kwargs)
 
     def get_all_repos(self):
         return list(self.config["repos"].keys())
diff --git a/utils/update_checkout/tests/test_clone.py b/utils/update_checkout/tests/test_clone.py
@@ -30,6 +30,17 @@ def test_simple_clone(self):
             repo_path = os.path.join(self.source_root, repo)
             self.assertTrue(os.path.isdir(repo_path))
 
+    def test_clone_with_additional_scheme(self):
+        output = self.call([self.update_checkout_path,
+                            '--config', self.config_path,
+                            '--config', self.additional_config_path,
+                            '--source-root', self.source_root,
+                            '--clone',
+                            '--scheme', 'extra'])
+
+        # Test that we're actually checking out the 'extra' scheme based on the output
+        self.assertIn(b"git checkout refs/heads/main", output)
+
 
 class SchemeWithMissingRepoTestCase(scheme_mock.SchemeMockTestCase):
     def __init__(self, *args, **kwargs):
diff --git a/utils/update_checkout/tests/test_dump.py b/utils/update_checkout/tests/test_dump.py
@@ -10,6 +10,8 @@
 #
 # ===----------------------------------------------------------------------===#
 
+import json
+
 from . import scheme_mock
 
 
@@ -25,9 +27,16 @@ def test_dump_hashes_json(self):
                    '--source-root', self.source_root,
                    '--clone'])
 
-        # Then dump the hashes. Just make sure we don't crash. We should test
-        # the output as well. But we should never crash.
-        self.call([self.update_checkout_path,
-                   '--config', self.config_path,
-                   '--source-root', self.source_root,
-                   '--dump-hashes'])
+        # Then dump the hashes.
+        output = self.call([self.update_checkout_path,
+                            '--config', self.config_path,
+                            '--source-root', self.source_root,
+                            '--dump-hashes'])
+        # The output should be valid JSON
+        result = json.loads(output)
+
+        # And it should have some basic properties we expect from this JSON
+        self.assertIn("https-clone-pattern", result)
+        self.assertEqual(result["repos"], scheme_mock.MOCK_CONFIG["repos"])
+        self.assertEqual(result["ssh-clone-pattern"], "DO_NOT_USE")
+        self.assertSetEqual(set(result["branch-schemes"].keys()), {"repro"})
diff --git a/utils/update_checkout/tests/test_merge_config.py b/utils/update_checkout/tests/test_merge_config.py
@@ -0,0 +1,75 @@
+# ===--- test_merge_config.py ---------------------------------------------===#
+#
+#  This source file is part of the Swift.org open source project
+#
+#  Copyright (c) 2025 Apple Inc. and the Swift project authors
+#  Licensed under Apache License v2.0 with Runtime Library Exception
+#
+#  See https:#swift.org/LICENSE.txt for license information
+#  See https:#swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+#
+# ===----------------------------------------------------------------------===#
+
+import unittest
+
+from update_checkout.update_checkout import merge_config, merge_no_duplicates
+
+
+class MergeTestCase(unittest.TestCase):
+    def test_no_duplicates(self):
+        self.assertEqual(merge_no_duplicates({}, {}), {})
+        self.assertEqual(merge_no_duplicates({"a": 1}, {"b": 2}), {"a": 1, "b": 2})
+        with self.assertRaises(ValueError):
+            merge_no_duplicates({"a": 1, "b": 2}, {"b": 3})
+
+    def test_merge_config(self):
+        default_config = {
+            "ssh-clone-pattern": "git@1",
+            "https-clone-pattern": "https://1",
+            "repos": {
+                "swift": {"remote": {"id": "swiftlang/swift"}},
+                "llvm-project": {"remote": {"id": "swiftlang/llvm-project"}},
+            },
+            "default-branch-scheme": "main",
+            "branch-schemes": {
+                "main": {
+                    "aliases": ["swift/main", "main", "stable/20240723"],
+                },
+            },
+        }
+
+        self.assertEqual(merge_config(default_config, {
+            "note": "this is machine generated or something",
+            "ssh-clone-pattern": "git@2",
+            "repos": {
+                "llvm-project": {"remote": {"id": "blah/llvm-project"}},
+                "swift-syntax": {"remote": {"id": "swiftlang/swift-syntax"}},
+            },
+            "default-branch-scheme": "bonus",
+            "branch-schemes": {
+                "bonus": {
+                    "aliases": ["bonus", "also-bonus"],
+                },
+            },
+        }), {
+            "ssh-clone-pattern": "git@2",
+            "https-clone-pattern": "https://1",
+            "repos": {
+                "swift": {"remote": {"id": "swiftlang/swift"}},
+                "llvm-project": {"remote": {"id": "blah/llvm-project"}},
+                "swift-syntax": {"remote": {"id": "swiftlang/swift-syntax"}},
+            },
+            "default-branch-scheme": "bonus",
+            "branch-schemes": {
+                "main": {
+                    "aliases": ["swift/main", "main", "stable/20240723"],
+                },
+                "bonus": {
+                    "aliases": ["bonus", "also-bonus"],
+                },
+            },
+            "note": "this is machine generated or something",
+        })
+
+        with self.assertRaises(ValueError):
+            merge_config(default_config, default_config)
diff --git a/utils/update_checkout/update_checkout/update_checkout.py b/utils/update_checkout/update_checkout/update_checkout.py
@@ -536,6 +536,40 @@ def print_repo_hashes(args, config):
         print("{:<35}: {:<35}".format(repo_name, repo_hash))
 
 
+def merge_no_duplicates(a: dict, b: dict) -> dict:
+    result = {**a}
+    for key, value in b.items():
+        if key in a:
+            raise ValueError(f"Duplicate scheme {key}")
+
+        result[key] = value
+    return result
+
+
+def merge_config(config: dict, new_config: dict) -> dict:
+    """
+    Merge two configs, with a 'last-wins' strategy.
+
+    The branch-schemes are rejected if they define duplicate schemes.
+    """
+
+    result = {**config}
+    for key, value in new_config.items():
+        if key == "branch-schemes":
+            # We reject duplicates here since this is the most conservative
+            # behavior, so it can be relaxed in the future.
+            # TODO: Another semantics might be nicer, define that as it is needed.
+            result[key] = merge_no_duplicates(config.get(key, {}), value)
+        elif key == "repos":
+            # The "repos" object is last-wins on a key-by-key basis
+            result[key] = {**config.get(key, {}), **value}
+        else:
+            # Anything else is just last-wins
+            result[key] = value
+
+    return result
+
+
 def validate_config(config):
     # Make sure that our branch-names are unique.
     scheme_names = config['branch-schemes'].keys()
@@ -669,9 +703,12 @@ def main():
     )
     parser.add_argument(
         "--config",
-        default=os.path.join(SCRIPT_DIR, os.pardir,
-                             "update-checkout-config.json"),
-        help="Configuration file to use")
+        help="""The configuration file to use. Can be specified multiple times,
+        each config will be merged together with a 'last-wins' strategy.
+        Overwriting branch-schemes is not allowed.""",
+        action="append",
+        default=[],
+        dest="configs")
     parser.add_argument(
         "--github-comment",
         help="""Check out related pull requests referenced in the given
@@ -734,8 +771,15 @@ def main():
     all_repos = args.all_repositories
     use_submodules = args.use_submodules
 
-    with open(args.config) as f:
-        config = json.load(f)
+    # Set the default config path if none are specified
+    if not args.configs:
+        default_path = os.path.join(SCRIPT_DIR, os.pardir,
+                                    "update-checkout-config.json")
+        args.configs.append(default_path)
+    config = {}
+    for config_path in args.configs:
+        with open(config_path) as f:
+            config = merge_config(config, json.load(f))
     validate_config(config)
 
     cross_repos_pr = {}
@@ -783,8 +827,10 @@ def main():
                           prefix="[swift] ")
 
                 # Re-read the config after checkout.
-                with open(args.config) as f:
-                    config = json.load(f)
+                config = {}
+                for config_path in args.configs:
+                    with open(config_path) as f:
+                        config = merge_config(config, json.load(f))
                 validate_config(config)
                 scheme_map = get_scheme_map(config, scheme_name)
 
