[PATCH] Re: 2 bugs in tag removal

[Osku Salerma]

On Sat, 8 Dec 2007, Osku Salerma wrote:

> On Sat, 8 Dec 2007, Matt Mackall wrote:
>> How about a parallel dict that tracks the source of each tag?
>
> I'll try implementing that.

Here's the parallel-dict implementation of fixing this. Note that this
version implements the more restrictive checks, i.e. does not allow
"deleting" a local tag that only exists as a global one.

# HG changeset patch
# User Osku Salerma <osku at iki.fi>
# Date 1197104242 -32400
# Node ID 492de6e0986e9560799f46fa5f3929540a3ebe8d
# Parent  feac5b0bf9bad2c125ebd5f3e133bcd46ecb8c7c
Properly check tag's existence as a local/global tag when removing it.

diff -r feac5b0bf9ba -r 492de6e0986e mercurial/commands.py
--- a/mercurial/commands.py	Wed Nov 28 13:58:31 2007 -0800
+++ b/mercurial/commands.py	Sat Dec 08 17:57:22 2007 +0900
@@ -2628,7 +2628,11 @@ def tag(ui, repo, name, rev_=None, **opt
          rev_ = opts['rev']
      message = opts['message']
      if opts['remove']:
-        if not name in repo.tags():
+        tagtype = repo.gettagtype(name)
+        found = (opts['local'] and tagtype == 'local') or \
+                (not opts['local'] and tagtype == 'global')
+
+        if not found:
              raise util.Abort(_('tag %s does not exist') % name)
          rev_ = nullid
          if not message:
diff -r feac5b0bf9ba -r 492de6e0986e mercurial/localrepo.py
--- a/mercurial/localrepo.py	Wed Nov 28 13:58:31 2007 -0800
+++ b/mercurial/localrepo.py	Sat Dec 08 17:57:22 2007 +0900
@@ -79,6 +79,7 @@ class localrepository(repo.repository):
              pass

          self.tagscache = None
+        self.tagstypecache = None
          self.branchcache = None
          self.nodetagscache = None
          self.filterpats = {}
@@ -198,8 +199,9 @@ class localrepository(repo.repository):
              return self.tagscache

          globaltags = {}
+        tagtypes = {}

-        def readtags(lines, fn):
+        def readtags(lines, fn, tagtype):
              filetags = {}
              count = 0

@@ -234,7 +236,9 @@ class localrepository(repo.repository):
              for k, nh in filetags.items():
                  if k not in globaltags:
                      globaltags[k] = nh
+                    tagtypes[k] = tagtype
                      continue
+
                  # we prefer the global tag if:
                  #  it supercedes us OR
                  #  mutual supercedes and it has a higher rank
@@ -246,31 +250,47 @@ class localrepository(repo.repository):
                      an = bn
                  ah.extend([n for n in bh if n not in ah])
                  globaltags[k] = an, ah
+                tagtypes[k] = tagtype

          # read the tags file from each head, ending with the tip
          f = None
          for rev, node, fnode in self._hgtagsnodes():
              f = (f and f.filectx(fnode) or
                   self.filectx('.hgtags', fileid=fnode))
-            readtags(f.data().splitlines(), f)
+            readtags(f.data().splitlines(), f, "global")

          try:
              data = util.fromlocal(self.opener("localtags").read())
              # localtags are stored in the local character set
              # while the internal tag table is stored in UTF-8
-            readtags(data.splitlines(), "localtags")
+            readtags(data.splitlines(), "localtags", "local")
          except IOError:
              pass

          self.tagscache = {}
+        self.tagstypecache = {}
          for k,nh in globaltags.items():
              n = nh[0]
              if n != nullid:
                  self.tagscache[k] = n
+                self.tagstypecache[k] = tagtypes[k]
          self.tagscache['tip'] = self.changelog.tip()

          return self.tagscache

+    def gettagtype(self, tagname):
+        '''
+        return the type of the given tag. result can be:
+
+        'local'  : a local tag
+        'global' : a global tag
+        None     : tag does not exist
+        '''
+
+        self.tags()
+ 
+        return self.tagstypecache.get(tagname)
+
      def _hgtagsnodes(self):
          heads = self.heads()
          heads.reverse()
@@ -553,6 +573,7 @@ class localrepository(repo.repository):
              if hasattr(self, a):
                  self.__delattr__(a)
          self.tagscache = None
+        self.tagstypecache = None
          self.nodetagscache = None

      def _lock(self, lockname, wait, releasefn, acquirefn, desc):
diff -r feac5b0bf9ba -r 492de6e0986e tests/test-tags
--- a/tests/test-tags	Wed Nov 28 13:58:31 2007 -0800
+++ b/tests/test-tags	Sat Dec 08 17:57:22 2007 +0900
@@ -126,3 +126,20 @@ hg tag -m 'retag rev 0' -fr 0 bar  # rev
  hg tag -m 'retag rev 0' -fr 0 bar  # rev 4 bar -> 0, but bar stays at 2
  echo % bar should still point to rev 2
  hg tags
+
+
+# test that removing global/local tags does not get confused when trying
+# to remove a tag of type X which actually only exists as a type Y
+cd ..
+hg init t5
+cd t5
+echo foo > foo
+hg add
+hg ci -m 'add foo'                 # rev 0
+
+hg tag -r 0 -l localtag
+hg tag --remove localtag
+
+hg tag -r 0 globaltag
+hg tag --remove -l globaltag
+exit 0
diff -r feac5b0bf9ba -r 492de6e0986e tests/test-tags.out
--- a/tests/test-tags.out	Wed Nov 28 13:58:31 2007 -0800
+++ b/tests/test-tags.out	Sat Dec 08 17:57:22 2007 +0900
@@ -71,3 +71,6 @@ bar                                2:72b
  % bar should still point to rev 2
  tip                                4:40af5d225513
  bar                                2:72b852876a42
+adding foo
+abort: tag localtag does not exist
+abort: tag globaltag does not exist

--
Osku Salerma