Why is it too late in the game for that? It's just a UI thing, and will work for
most people anyway. People who know what they're doing could still go through
it. And/or we could still allow it on --force, or something.

On Wed, 2008-09-10 at 07:47 +0000, Dirkjan Ochtman wrote:
> Dirkjan Ochtman <dirkjan@ochtman.nl> added the comment:
> 
> Actually, I think we should disallow creating tags to other branches (that is,
> tagged revision must be ancestor of tag revision).

An interesting notion. Probably too late in the game for that though.

>  Otherwise, there are cases
> that start to loudly complain about inexistent tags when you only have part of
> the DAG.

I think those warnings are a bit bogus.

I don't mind, and there are some cases, especially when a lot of tags are
created, where it is very nice to have them out on their own disconnected
branch.  I think that perhaps offering a way to mute warnings about tags that
refer to nodes that don't exist might be a better option.

I think tags are meta-data and I'm not convinced that meta-data belongs in the
same DAG as actual repository revisions.

Actually, I think we should disallow creating tags to other branches (that is,
tagged revision must be ancestor of tag revision). Otherwise, there are cases
that start to loudly complain about inexistent tags when you only have part of
the DAG.

I backed it out, back to chatting.

What would be a good option name for 3 ?

Back it unless and until there is a less controversial improvement. #3 would be
ideal for the use case of making automated tags on a server, but it would mean
that 'hg tag' would have to be capable of operating directly on the repository
without a checkout.

Jesse, do you object ?

On Tue, 2008-09-09 at 20:55 +0000, Benoit Boissinot wrote:
> Benoit Boissinot <bboissin@gmail.com> added the comment:
> 
> Then should I back it out and close this ?

I think so, unless someone wants to defend the contrary position.

Then should I back it out and close this ?

Indeed I did. No checkout == null rev checked out. Once you know that, there's
no longer any surprise. And it may actually be sensible to keep tags in their
own disconnected branch (which this patch prevents).

I seem to recall that mpm objected to the desirability of the patched behavior
in the past? Don't remember details now.

pushed to crew (3d54cf97598d)

I think this patch looks good.

attached patch with tests.

There's probably a better way to do this (I don't know the new context API enough :)

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -221,7 +221,11 @@
                 raise util.Abort(_('working copy of .hgtags is changed '
                                    '(please commit .hgtags manually)'))
 
-        self._tag(names, node, message, local, user, date)
+        parents = self[None].parents()
+        parent = None
+        if len(parents) == 1 and parents[0].node() == nullid:
+            parent = self['tip'].node()
+        self._tag(names, node, message, local, user, date, parent=parent)
 
     def tags(self):
         '''return a mapping of tag to node'''

I think fix 1 would be fine.

BTW I ran across this when trying to do a server-side clone:

cd hgwebdir
hg -R devel tag release-1.0
hg clone -U -r release-1.0 devel rel10
hg -R rel10 tag release-1.0

Since the published repos on the server do not have checkouts, the tag steps
produce the wrong results.

---%<---
$ hg clone -U http://selenic.com/repo/hg repo
requesting all changes
adding changesets
adding manifests
adding file changes
added 5793 changesets with 10852 changes to 790 files
$ hg -R repo tag whatever
$ hg -R repo log -l2
changeset:   5793:52ac65f4407b
tag:         tip
parent:      -1:000000000000
user:        Jesse Glick <jesse.glick@sun.com>
date:        Mon Jan 07 11:54:35 2008 -0500
summary:     Added tag whatever for changeset 956e01c31914

changeset:   5792:956e01c31914
tag:         whatever
user:        Kevin Christen <kevin.christen@gmail.com>
date:        Thu Jan 03 20:27:32 2008 -0600
summary:     color extension: change from GPL3 to 2
---%<---

Note that the new revision containing the change to .hgtags is a child of the
null revision! Of course I wanted to commit it to the default branch as a child
of the tagged revision. If I ran the above command and neglected to verify the
result using hg log before pushing, the next person to do a (regular) clone will
see what looks like a busted repository:

---%<---
$ hg clone repo repo2
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ ls repo2
$ ls -a repo2
.  ..  .hg  .hgtags
---%<---

If you do this, the fix is to run

---%<---
$ cd repo2
$ hg up -C -r5792
700 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge
merging .hgtags
# resolve conflict in merge tool...
0 files updated, 1 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
$ hg ci -m fixed
$ hg log -l3
changeset:   5794:0297b2d17e56
tag:         tip
parent:      5792:956e01c31914
parent:      5793:52ac65f4407b
user:        Jesse Glick <jesse.glick@sun.com>
date:        Mon Jan 07 12:15:48 2008 -0500
summary:     fixed

changeset:   5793:52ac65f4407b
parent:      -1:000000000000
user:        Jesse Glick <jesse.glick@sun.com>
date:        Mon Jan 07 11:54:35 2008 -0500
summary:     Added tag whatever for changeset 956e01c31914

changeset:   5792:956e01c31914
tag:         whatever
user:        Kevin Christen <kevin.christen@gmail.com>
date:        Thu Jan 03 20:27:32 2008 -0600
summary:     color extension: change from GPL3 to 2
---%<---

which works but is a little subtle.

I would recommend that hg tag when run on a repository with no checked-out
revision behave in one of these ways:

1. Commit as a child of tip. (best IMO)

2. Abort with an error message, unless perhaps -f is used.

3. Accept a new parameter specifying the parent revision to commit from.

If desired, I can try to implement one of these options.

BTW the code in localrepo.py is confusing. _tag takes a 'parent' parameter, yet
this method is only called from one place (tag), which does not pass that
parameter (nor the 'extra' param). Reserved for future expansion, or should
self._tag be called with parent=tip if there is no checkout (i.e. fix #1)?