Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure repos to ignore tags, and force branches when rev-parsing #922

Open
xmo-odoo opened this issue Aug 1, 2024 · 0 comments
Open
Labels

Comments

@xmo-odoo
Copy link
Collaborator

xmo-odoo commented Aug 1, 2024

odoo/o-spreadsheet#4722 got forward-ported to 17.0 as 66 commits instead of the one (odoo/o-spreadsheet#4740).

edit: also odoo/o-spreadsheet#4749 so it happened twice on spreadsheet porting from 16 to 17 and a quick check didn't reveal anything in odoo/odoo or odoo/enterprise, so might be something in o-spreadsheet which confuses the WC-less runbot forward porting thingie

edit 2: also odoo/o-spreadsheet#4815 which doesn't have conflicts so that's not where the issue is.

As it turns out the issue is that o-spreadsheet uses a shit-ton of tags, and they fucked up the 17.0.0 tag: it was created as 17.0, then deleted. Except the mergebot managed to fetch it before that and git does not prune tags by default.

And when a ref is ambiguous rev-parse favors tags

When ambiguous, a is disambiguated by taking the first match in the following rules:

  1. If $GIT_DIR/ 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/ if it exists;
  3. otherwise, refs/tags/ if it exists;
  4. otherwise, refs/heads/ if it exists;
  5. otherwise, refs/remotes/ if it exists;
  6. otherwise, refs/remotes//HEAD if it exists.

So git would resolve 17.0 to refs/tags/17.0, it would complain that "refname '17.0' is ambiguous." but that pretty much got ignored (I couldn't even find the info in journalctl).

The information was in the logs but I didn't notice it:

runbot_merge.pull_requests(234982,): copy 1 commits to 17.0 (5d60210a896b51c244af46c03978a62f7cf892cd)

which is the commit for the tag 17.0, completely different of and unrelated to the branch of the same name. As such the mergebot would dutifully rebase the commits from the source PR onto that tag, and create the conflict commits over there too (for the same reasons).


  • git fetch: By default, any tag that points into the histories being fetched is also fetched; the effect is to fetch tags that point at branches that you are interested in. This default behavior can be changed by using the --no-tags options or by configuring remote.<name>.tagOpt (so remote.origin.tagOpt = --no-tags)
  • git rev-parse: force refs/heads/ prefix branches

Ancillary notes from a bit of testing:

  • fetching an exact commit does not seem to retrieve tags (?)
  • fetching a ref by full refspec does fetch tags regardless
@xmo-odoo xmo-odoo added mergebot blocked to check once in a while: third-party factors currently make this not possible labels Aug 1, 2024
@xmo-odoo xmo-odoo removed the blocked to check once in a while: third-party factors currently make this not possible label Aug 9, 2024
@xmo-odoo xmo-odoo changed the title Fix incorrect commits selection when pushing conflicts on failed forward ports Configure repos to ignore tags, and force branches when rev-parsing Aug 9, 2024
xmo-odoo added a commit that referenced this issue Sep 6, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: done
Development

No branches or pull requests

1 participant