Page MenuHomePhabricator

Branch script doesn't respect `branchpoint` when creating submodules, resulting in "implicit merges" error from gerrit
Open, MediumPublicFeature

Description

Actions

Jeena and I were working to branch MediaWiki for 1.36. We ran ./branch.py REL1_36 --core --branchpoint e497482ee4e0da58d2f3f6ec996f8b20f64b2d80 --core-version 1.36.0-beta --task T279456 to branch MediaWiki core and create a patch.

Expected outcome

The patch is created and pushed to gerrit.

Actual outcome

Gerrit rejects the patch as having implicit merges (the difference between REL1_36 and master at the time we ran it).

Analysis

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

Event Timeline

thcipriani added a subscriber: thcipriani.

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

We cut the branch successfully, was there a workaround?

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

We cut the branch successfully, was there a workaround?

Yes, I manually created the patch. Ick.

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

We cut the branch successfully, was there a workaround?

Yes, I manually created the patch. Ick.

Boo. OK, we should fix this before next release :(

Ping. It'd be really really nice to get this fixed before we hit the bug again on Tuesday for T289586. Sorry for the very-short-notice-for-me-to-remember. (Did we decide that T283883 was caused by this for certain?)

(Alternatively, I can just stay around and run this at the same time as the ReleaseBranchBot.)

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

I'm not sure I understand this. For do_core_work the script seems to:

  1. Create the mwcore branch REL1_36 from --branch-point via gerrit api
  2. Clone mwcore
  3. Checkout the newly created branch (REL1_36)
  4. Add submodules
  5. Push patch for review

which seems right...

What branchpoints did you pass for submodules?

The do_core_work method runs git('checkout', '-B', branch) against (implicitly) master from with clone('mediawiki/core'), rather than from the branch point (which the branch method doesn't pass through).

I'm not sure I understand this. For do_core_work the script seems to:

  1. Create the mwcore branch REL1_36 from --branch-point via gerrit api
  2. Clone mwcore
  3. Checkout the newly created branch (REL1_36)
  4. Add submodules
  5. Push patch for review

which seems right...

What branchpoints did you pass for submodules?

--branchpoint e497482ee4e0da58d2f3f6ec996f8b20f64b2d80 is the branch point of core which the script is meant to translate into the right branch point for each sub-module, right?

What branchpoints did you pass for submodules?

--branchpoint e497482ee4e0da58d2f3f6ec996f8b20f64b2d80 is the branch point of core which the script is meant to translate into the right branch point for each sub-module, right?

hrm, unless gerrit is smarter than the script, the script looks for the branchpoint in .settings.yaml and settings.yaml for the branchpoint (manual_branch_points.revisions.repository) and if it comes up empty it uses the gerrit api to make a new branch passing in whatever is used as branchpoint. If gerrit doesn't do anything smart, it may just branch from whatever branch is set as default (master for pretty much all bundled repos). Does that seem like it would explain the implicit merges thing?

What branchpoints did you pass for submodules?

--branchpoint e497482ee4e0da58d2f3f6ec996f8b20f64b2d80 is the branch point of core which the script is meant to translate into the right branch point for each sub-module, right?

hrm, unless gerrit is smarter than the script, the script looks for the branchpoint in .settings.yaml and settings.yaml for the branchpoint (manual_branch_points.revisions.repository) and if it comes up empty it uses the gerrit api to make a new branch passing in whatever is used as branchpoint. If gerrit doesn't do anything smart, it may just branch from whatever branch is set as default (master for pretty much all bundled repos). Does that seem like it would explain the implicit merges thing?

That would explain everything, yes. Manually adding dozens of branch points to settings.yaml is going to be very tedious. But previously the (previous?) script did all the bumps for us, didn't it? It's been so long though…

What branchpoints did you pass for submodules?

--branchpoint e497482ee4e0da58d2f3f6ec996f8b20f64b2d80 is the branch point of core which the script is meant to translate into the right branch point for each sub-module, right?

hrm, unless gerrit is smarter than the script, the script looks for the branchpoint in .settings.yaml and settings.yaml for the branchpoint (manual_branch_points.revisions.repository) and if it comes up empty it uses the gerrit api to make a new branch passing in whatever is used as branchpoint. If gerrit doesn't do anything smart, it may just branch from whatever branch is set as default (master for pretty much all bundled repos). Does that seem like it would explain the implicit merges thing?

That would explain everything, yes. Manually adding dozens of branch points to settings.yaml is going to be very tedious. But previously the (previous?) script did all the bumps for us, didn't it? It's been so long though…

That's a good question: @mmodell may have a better memory of ^ than me?

OK, so I changed the command from giving an explicit branch point hash to just master for REL1_37 and I think it Just Worked™?

mmodell raised the priority of this task from Low to Medium.Sep 14 2021, 5:09 AM

That's a good question: @mmodell may have a better memory of ^ than me?

I don't know if --branchpoint ever worked correctly and I don't think I ever tried it. I believe it's a holdover from an older version of the script that is from before my time here.

thcipriani changed the subtype of this task from "Task" to "Feature Request".