Page MenuHomePhabricator

Never use "{branches}" in Mercurial
ClosedPublic

Authored by epriestley on Jun 10 2014, 2:10 PM.
Tags
None
Referenced Files
F14063656: D9453.diff
Mon, Nov 18, 7:41 PM
F14060117: D9453.diff
Sun, Nov 17, 11:16 PM
F14048041: D9453.id23142.diff
Thu, Nov 14, 6:35 AM
F14046555: D9453.id.diff
Wed, Nov 13, 9:53 PM
F14045768: D9453.diff
Wed, Nov 13, 10:36 AM
F14045567: D9453.diff
Wed, Nov 13, 6:34 AM
F14034597: D9453.diff
Sun, Nov 10, 1:12 AM
F13979445: D9453.diff
Oct 19 2024, 4:36 AM
Subscribers

Details

Summary

Fixes T5304. Mercurial features a "{branches}" template keyword, documented as:

branches      List of strings. The name of the branch on which the
              changeset was committed. Will be empty if the branch name
              was default.

At some time long in the past, I misinterpreted this to mean "list of branches where the branch head is a descendant of the commit". It is more like "list of zero or one elements, possibly containing the name of the branch the commit was originally made to, if that branch was not 'default'".

In fact, it seems like this is because a very long time in the past, Mercurial worked roughly like I expected:

Ages ago (2005), we had a very different and ultimately unworkable
approach to named branches that worked vaguely like .hgtags and allowed
multiple branch names per revision.

http://marc.info/?l=mercurial-devel&m=129883069414855

This appears to be deprecated in modern Mercurial (it's not in the modern web documentation) although I can't find a commit about it so maybe that's just a documentation issue.

In any case, {branches} seems to never be useful: {branch} provides the same information without the awkward "default-if-empty" case.

Switch from {branches} to either {branch} (where that's good enough, notably in the hook engine) or (descendants(%s) and head()), which is equivalent to --contains in Git.

This fixes pushing to branches with spaces in their names, and makes the "Branches" / "Contains" queries moderately more consistent.

Test Plan
  • Pushed to a Mercurial branch with a space in it.
  • Viewed list of branches in a Mercurial repository.
  • Viewed containing branches of a Mercurial commit in Diffusion.

Diff Detail

Repository
rP Phabricator
Branch
neverbranhces
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 957
Build 957: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Never use "{branches}" in Mercurial.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php
87–91

This also fixes the issue where we'll return branch heads which do not actually descend from the commit.

btrahan edited edge metadata.
This revision is now accepted and ready to land.Jun 20 2014, 5:17 PM
epriestley updated this revision to Diff 23142.

Closed by commit rP5660684d7f44 (authored by @epriestley).