| Submitter | Yuya Nishihara |
|---|---|
| Date | May 7, 2013, 10:45 p.m. |
| Message ID | <f42a6337cac2c3920fb7.1367966756@gimlet> |
| Download | mbox | patch |
| Permalink | /patch/1589/ |
| State | Accepted |
| Commit | 12dbdd348bb0977366200bf96cb6d2afa85faf13 |
| Headers | show |
Comments
On Wed, May 08, 2013 at 07:45:56AM +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1367938143 -32400 > # Tue May 07 23:49:03 2013 +0900 > # Branch stable > # Node ID f42a6337cac2c3920fb7587797ada022c1141182 > # Parent 0a12e5f3a979ee302dc10647483200df00a105ab > branch: strip whitespace before testing known branch name LGTM, but my paranoia wants either mpm to see this and ack it, or another crew member to do so before queueing this for stable, since it is technically a behavior change (even though it strikes me as a bug fix in light of 3d0a9c8d7184.) > > Because dirstate._branch() strips leading/trailing spaces from .hg/branch, > "hg branch ' foo '" should abort if branch "foo" exists in another head. > > tag command had a similar bug and fixed by 3d0a9c8d7184. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -943,6 +943,9 @@ def branch(ui, repo, label=None, **opts) > > Returns 0 on success. > """ > + if label: > + label = label.strip() > + > if not opts.get('clean') and not label: > ui.write("%s\n" % repo.dirstate.branch()) > return > diff --git a/tests/test-branches.t b/tests/test-branches.t > --- a/tests/test-branches.t > +++ b/tests/test-branches.t > @@ -68,6 +68,18 @@ invalid characters > abort: '\n' cannot be used in a name > [255] > > +trailing or leading spaces should be stripped before testing duplicates > + > + $ hg branch 'b ' > + abort: a branch of the same name already exists > + (use 'hg update' to switch to it) > + [255] > + > + $ hg branch ' b' > + abort: a branch of the same name already exists > + (use 'hg update' to switch to it) > + [255] > + > verify update will accept invalid legacy branch names > > $ hg init test-invalid-branch-name > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Thu, 9 May 2013 14:58:43 -0500, Kevin Bullock wrote: > On 7 May 2013, at 5:45 PM, Yuya Nishihara wrote: > > # HG changeset patch > > # User Yuya Nishihara <yuya@tcha.org> > > # Date 1367938143 -32400 > > # Tue May 07 23:49:03 2013 +0900 > > # Branch stable > > # Node ID f42a6337cac2c3920fb7587797ada022c1141182 > > # Parent 0a12e5f3a979ee302dc10647483200df00a105ab > > branch: strip whitespace before testing known branch name > > > > Because dirstate._branch() strips leading/trailing spaces from .hg/branch, > > "hg branch ' foo '" should abort if branch "foo" exists in another head. > > This should be done in scmutil.checknewlabel() if possible. It's already done > for tags, and should apply to bookmarks as well. It seems checknewlabel() is a bit different. checknewlabel() tests invalid label, which is mandatory check. But in this case, it's valid to have multiple branch heads of the same name. This patch tries to address the following problem: - "hg branch ' foo '" creates the branch "foo" on next commit. - but it bypasses "label in repo.branchmap()", and thus doesn't abort even if there exists the branch "foo" in another head. Regards,
Patch
diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -943,6 +943,9 @@ def branch(ui, repo, label=None, **opts) Returns 0 on success. """ + if label: + label = label.strip() + if not opts.get('clean') and not label: ui.write("%s\n" % repo.dirstate.branch()) return diff --git a/tests/test-branches.t b/tests/test-branches.t --- a/tests/test-branches.t +++ b/tests/test-branches.t @@ -68,6 +68,18 @@ invalid characters abort: '\n' cannot be used in a name [255] +trailing or leading spaces should be stripped before testing duplicates + + $ hg branch 'b ' + abort: a branch of the same name already exists + (use 'hg update' to switch to it) + [255] + + $ hg branch ' b' + abort: a branch of the same name already exists + (use 'hg update' to switch to it) + [255] + verify update will accept invalid legacy branch names $ hg init test-invalid-branch-name