From 60188063f8857991014463020bfc0cd884e06187 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 6 Sep 2024 15:09:08 +0200 Subject: [PATCH] [FIX] *: ensure I don't get bollocked up again by tags Today (or really a month ago) I learned: when giving git a symbolic ref (e.g. a ref name), if it's ambiguous then 1. If `$GIT_DIR/$name` exists, that is what you mean (this is usually useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD`, `MERGE_HEAD`, `REBASE_HEAD`, `REVERT_HEAD`, `CHERRY_PICK_HEAD`, `BISECT_HEAD` and `AUTO_MERGE`) 2. otherwise, `refs/$name` if it exists 3. otherwise, `refs/tags/$name` if it exists 4. otherwise, `refs/heads/$name` if it exists 5. otherwise, `refs/remotes/$name` if it exists 6. otherwise, `refs/remotes/$name/HEAD` if it exists This means if a tag and a branch have the same name and only the name is provided (not the full ref), git will select the tag, which gets very confusing for the mergebot as it now tries to rebase onto the tag (which because that's not fun otherwise was not even on the branch of the same name). Fix by providing full refs to `rev-parse` when trying to retrieve the head of the target branches. And as a defense in depth opportunity, also exclude tags when fetching refs by spec: apparently fetching a specific commit does not trigger the retrieval of tags, but any sort of spec will see the tags come along for the ride even if the tags are not in any way under the fetched ref e.g. `refs/heads/*` will very much retrieve the tags despite them being located at `refs/tags/*`. Fixes #922 --- forwardport/models/project.py | 5 +++-- runbot_merge/models/project_freeze/__init__.py | 2 +- runbot_merge/models/stagings_create.py | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index c3d4806d..0031b61b 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -362,7 +362,8 @@ stderr: {err} """ - target_head = source.stdout().rev_parse(target_branch.name).stdout.decode().strip() + target_head = source.stdout().rev_parse(f"refs/heads/{target_branch.name}")\ + .stdout.decode().strip() commit = conf.commit_tree( tree=tree.stdout.decode().splitlines(keepends=False)[0], parents=[target_head], @@ -386,7 +387,7 @@ stderr: logger = _logger.getChild(str(self.id)).getChild('cherrypick') # target's head - head = repo.stdout().rev_parse(branch).stdout.decode().strip() + head = repo.stdout().rev_parse(f"refs/heads/{branch}").stdout.decode().strip() commits = self.commits() logger.info( diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py index 0a39ceae..7af53f27 100644 --- a/runbot_merge/models/project_freeze/__init__.py +++ b/runbot_merge/models/project_freeze/__init__.py @@ -217,7 +217,7 @@ class FreezeWizard(models.Model): for r in self.project_id.repo_ids } for repo, copy in repos.items(): - copy.fetch(git.source_url(repo), '+refs/heads/*:refs/heads/*') + copy.fetch(git.source_url(repo), '+refs/heads/*:refs/heads/*', no_tags=True) all_prs = self.release_pr_ids.pr_id | self.bump_pr_ids.pr_id for pr in all_prs: repos[pr.repository].fetch( diff --git a/runbot_merge/models/stagings_create.py b/runbot_merge/models/stagings_create.py index 75464045..08734958 100644 --- a/runbot_merge/models/stagings_create.py +++ b/runbot_merge/models/stagings_create.py @@ -244,7 +244,8 @@ def staging_setup( # be hooked only to "proper" remote-tracking branches # (in `refs/remotes`), it doesn't seem to work here f'+refs/heads/{target.name}:refs/heads/{target.name}', - *(pr.head for pr in by_repo.get(repo, [])) + *(pr.head for pr in by_repo.get(repo, [])), + no_tags=True, ) original_heads[repo] = head staging_state[repo] = StagingSlice(gh=gh, head=head, repo=source.stdout().with_config(text=True, check=False))