Support specifying a pin for merges

Author: janverb

git_aggregator/config.py

@@ -58,19 +58,23 @@ def get_repos(config, force=False):
                 try:
                     # Assume parts is a str
                     parts = merge.split(' ')
-                    if len(parts) != 2:
+                    if len(parts) not in {2, 3}:
                         raise ConfigException(
                             '%s: Merge must be formatted as '
-                            '"remote_name ref".' % directory)
+                            '"remote_name ref [pin]".' % directory)
                     merge = {
                         "remote": parts[0],
                         "ref": parts[1],
                     }
+                    if len(parts) == 3:
+                        merge["pin"] = parts[2]
                 except AttributeError:
                     # Parts is a dict
                     try:
                         merge["remote"] = str(merge["remote"])
                         merge["ref"] = str(merge["ref"])
+                        if merge.get("pin"):
+                            merge["pin"] = str(merge["pin"])
                     except KeyError:
                         raise ConfigException(
                             '%s: Merge lacks mandatory '

git_aggregator/repo.py

@@ -275,31 +275,45 @@ def fetch(self):
         It returns a dict structure associating each (remote, ref) to their
         SHA in local repository.
         """
-        merges_requested = [(m["remote"], m["ref"])
-                            for m in self.merges]
         basecmd = ("git", "fetch")
         logger.info("Fetching required remotes")
         fetch_heads = {}
         ls_remote_refs = collections.defaultdict(list)  # to ls-query
-        while merges_requested:
-            remote, ref = merges_requested[0]
-            merges_requested = merges_requested[1:]
+        for merge in self.merges:
+            remote = merge["remote"]
+            ref = merge["ref"]
+            pin = merge.get("pin")
             cmd = (
                 basecmd +
                 self._fetch_options({"remote": remote, "ref": ref}) +
                 (remote,))
             if remote not in self.fetch_all:
                 cmd += (ref, )
             else:
+                if pin:
+                    # Probably solvable, but a little too tricky for me to
+                    # figure out right now
+                    raise GitAggregatorException(
+                        "Cannot use fetch_all with pin"
+                    )
                 ls_remote_refs[remote].append(ref)
             self.log_call(cmd, cwd=self.cwd)
-            with open(os.path.join(self.cwd, ".git", "FETCH_HEAD"), "r") as f:
-                for line in f:
-                    fetch_head, for_merge, _ = line.split("\t")
-                    if for_merge == "not-for-merge":
-                        continue
-                    break
-            fetch_heads[(remote, ref)] = fetch_head
+            if pin:
+                try:
+                    fetch_heads[(remote, ref)] = self.rev_parse(pin)
+                except Exception:
+                    logger.error(
+                        "Could not find pin %r after fetching %r", pin, ref
+                    )
+                    raise
+            else:
+                with open(os.path.join(self.cwd, ".git", "FETCH_HEAD")) as f:
+                    for line in f:
+                        fetch_head, for_merge, _ = line.split("\t")
+                        if for_merge == "not-for-merge":
+                            continue
+                        break
+                fetch_heads[(remote, ref)] = fetch_head
         if self.fetch_all:
             if self.fetch_all is True:
                 remotes = self.remotes
@@ -311,31 +325,16 @@ def fetch(self):
                     remote["url"],
                     ls_remote_refs[remote["name"]])
                 for _, ref, sha in refs:
-                    if (remote["name"], ref) in merges_requested:
-                        merges_requested.remove((remote["name"], ref))
                     fetch_heads[(remote["name"], ref)] = sha
-        if len(merges_requested):
-            # Last case: our ref is a sha and remote git repository does
-            # not support querying commit directly by SHA. In this case
-            # we need just to check if ref is actually SHA, and if we have
-            # this SHA locally.
-            for remote, ref in merges_requested:
-                if not re.search("[0-9a-f]{4,}", ref):
-                    raise ValueError("Could not resolv ref %r on remote %r"
-                                     % (ref, remote))
-            valid_local_shas = self.log_call(
-                ['git', 'rev-parse', '-v'] + [sha
-                                              for _r, sha in merges_requested],
-                cwd=self.cwd, callwith=subprocess.check_output
-            ).strip().splitlines()
-            for remote, sha in merges_requested:
-                if sha not in valid_local_shas:
-                    raise ValueError(
-                        "Could not find SHA ref %r after fetch on remote %r"
-                        % (ref, remote))
-                fetch_heads[(remote["name"], sha)] = sha
         return fetch_heads
 
+    def rev_parse(self, ref):
+        return self.log_call(
+            ["git", "rev-parse", "--verify", ref],
+            callwith=subprocess.check_output,
+            cwd=self.cwd,
+        ).strip()
+
     def push(self):
         remote = self.target['remote']
         branch = self.target['branch']
@@ -384,11 +383,7 @@ def _switch_to_branch(self, branch_name, ref=None):
         logger.info("Switch to branch %s", branch_name)
         cmd = ['git', 'checkout', '-B', branch_name]
         if ref is not None:
-            sha1 = self.log_call(
-                ['git', 'rev-parse', ref],
-                callwith=subprocess.check_output,
-                cwd=self.cwd).strip()
-            cmd.append(sha1)
+            cmd.append(self.rev_parse(ref))
         self.log_call(cmd, cwd=self.cwd)
 
     def _execute_shell_command_after(self):

tests/test_config.py

@@ -27,7 +27,7 @@ def test_load(self):
     merges:
         - oca 8.0
         - oca refs/pull/105/head
-        - oca refs/pull/106/head
+        - oca refs/pull/106/head 1234abcd
     target: acsone aggregated_branch_name
         """
         repos = config.get_repos(self._parse_config(config_yaml))
@@ -44,7 +44,8 @@ def test_load(self):
              'defaults': {},
              'merges': [{'ref': '8.0', 'remote': 'oca'},
                         {'ref': 'refs/pull/105/head', 'remote': 'oca'},
-                        {'ref': 'refs/pull/106/head', 'remote': 'oca'}],
+                        {'ref': 'refs/pull/106/head', 'remote': 'oca',
+                         'pin': '1234abcd'}],
              'remotes': [],
              'shell_command_after': [],
              'target': {'branch': 'aggregated_branch_name',
@@ -75,6 +76,7 @@ def test_load_defaults(self):
                     -
                         remote: oca
                         ref: refs/pull/106/head
+                        pin: 1234abcd
                 target: acsone aggregated_branch_name
         """)
         repos = config.get_repos(self._parse_config(config_yaml))
@@ -91,7 +93,8 @@ def test_load_defaults(self):
              'defaults': {'depth': 1},
              'merges': [{'ref': '8.0', 'remote': 'oca', 'depth': 1000},
                         {'ref': 'refs/pull/105/head', 'remote': 'oca'},
-                        {'ref': 'refs/pull/106/head', 'remote': 'oca'}],
+                        {'ref': 'refs/pull/106/head', 'remote': 'oca',
+                         'pin': '1234abcd'}],
              'remotes': [],
              'shell_command_after': [],
              'target': {'branch': 'aggregated_branch_name',
@@ -201,7 +204,7 @@ def test_load_merges_exception(self):
         self.assertEqual(
             ex.exception.args[0],
             '/product_attribute: Merge must be formatted as '
-            '"remote_name ref".')
+            '"remote_name ref [pin]".')
         config_yaml = """
 /product_attribute:
     remotes:

tests/test_repo.py

@@ -451,3 +451,47 @@ def test_multithreading(self):
 
         self.assertTrue(os.path.isfile(os.path.join(repo3_dir, 'tracked')))
         self.assertTrue(os.path.isfile(os.path.join(repo3_dir, 'tracked2')))
+
+    def test_pinned_base(self):
+        remotes = [{
+            'name': 'r1',
+            'url': self.url_remote1
+        }]
+        merges = [{
+            'remote': 'r1',
+            'ref': 'master',
+            'pin': self.commit_1_sha[:8]
+        }]
+        target = {
+            'remote': 'r1',
+            'branch': 'agg1'
+        }
+        repo = Repo(self.cwd, remotes, merges, target)
+        repo.aggregate()
+        last_rev = git_get_last_rev(self.cwd)
+        self.assertEqual(last_rev, self.commit_1_sha)
+
+    def test_pinned_merge(self):
+        remotes = [{
+            'name': 'r1',
+            'url': self.url_remote1
+        }, {
+            'name': 'r2',
+            'url': self.url_remote2
+        }]
+        merges = [{
+            'remote': 'r1',
+            'ref': 'tag2'
+        }, {
+            'remote': 'r2',
+            'ref': self.commit_3_sha,
+            'pin': self.commit_1_sha[:8]
+        }]
+        target = {
+            'remote': 'r1',
+            'branch': 'agg'
+        }
+        repo = Repo(self.cwd, remotes, merges, target)
+        repo.aggregate()
+        last_rev = git_get_last_rev(self.cwd)
+        self.assertEqual(last_rev, self.commit_2_sha)