[PATCH 1 of 1 RFC] tag: abort if not at a branch head

[Kevin Bullock]

On 6 Dec 2010, at 6:07 PM, Adrian Buehlmann wrote:

> On 2010-12-06 20:56, Kevin Bullock wrote:
>> # HG changeset patch
>> # User Kevin Bullock <kbullock at ringworld.org>
>> # Date 1291665075 21600
>> # Node ID f52e752c6d5f90352bb29ba6c2543656f6e4531e
>> # Parent  8c6b7a5f38c4585b1b2c2ceaffa8023f36a18479
>> tag: abort if not at a branch head
>> 
>> Since it's usually only desirable to make tag commits on top of branch
>> heads, abort if the working dir parent is not a branch head. -f/--force
>> may be passed to commit at a non-head anyway.
>> 
>> Does not abort if working dir parent is a named branch head but not a
>> topological head.
>> 
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -3665,16 +3665,23 @@
>> 
>>     Tags are used to name particular revisions of the repository and are
>>     very useful to compare different revisions, to go back to significant
>> -    earlier versions or to mark branch points as releases, etc.
>> +    earlier versions or to mark branch points as releases, etc. Changing
>> +    an existing tag is normally disallowed; use -f/--force to override.
>> 
>>     If no revision is given, the parent of the working directory is
>>     used, or tip if no revision is checked out.
>> 
>>     To facilitate version control, distribution, and merging of tags,
>> -    they are stored as a file named ".hgtags" which is managed
>> -    similarly to other project files and can be hand-edited if
>> -    necessary. The file '.hg/localtags' is used for local tags (not
>> -    shared among repositories).
>> +    they are stored as a file named ".hgtags" which is managed similarly
>> +    to other project files and can be hand-edited if necessary. This
>> +    also means that tagging creates a new commit. The file
>> +    ".hg/localtags" is used for local tags (not shared among
>> +    repositories).
>> +
>> +    Tag commits are usually made at the head of a branch. If the parent
>> +    of the working directory is not a branch head, :hg:`tag` aborts; use
>> +    -f/--force to force the tag commit to be based on a non-head
>> +    changeset.
>> 
>>     See :hg:`help dates` for a list of formats valid for -d/--date.
>> 
>> @@ -3713,6 +3720,11 @@
>>             # we don't translate commit messages
>>             message = 'Removed tag %s' % ', '.join(names)
>>     elif not opts.get('force'):
>> +        wctx = repo[None]
>> +        bheads = repo.branchheads()
>> +        if bheads and not [x for x in wctx.parents()
>> +                           if x.node() in bheads]:
>> +            raise util.Abort(_('not at a branch head (use -f to force)'))
> 
> (1) [x for ..] looks convoluted, '... in repo.branchheads():' should
>    do (compare with commit)

The first check, 'if bheads', ensures that tagging an empty repo still works. Don't ask me why that works, but it currently does. (I lifted that conditional with slight modification from localrepo.commit()'s check to see if it should show 'created new head'.) It will still abort, though, when tagging on the null revision (i.e. `hg up null; hg tag foo`—and I have a test that shows both these cases now); hg's current behavior is to silently create a new root commit, which I thought we'd want to avoid.

> (2) for x in wctx.parents() looks weird, tagging a merge should give a
>    nice error message. Maybe "abort: outstanding uncommitted merges"
>    like we already have elsewhere -> create a separate first patch
>    to fix this.
> 
> (3) Did you think about local tags?

Apparently not, because when I tested adding a local tag (which of course never creates a commit) on a non-head revision, it aborted. I've got a fix for that.

>>         for n in names:
>>             if n in repo.tags():
>>                 raise util.Abort(_('tag \'%s\' already exists '
> 
> Maybe this check should be done first.

Fair point.

I'll re-send with the discussed fixes.

pacem in terris / mir / shanti / salaam / heiwa
Kevin R. Bullock