[PATCH] Re: 2 bugs in tag removal

Osku Salerma osku at iki.fi
Sun Dec 9 01:24:31 UTC 2007


On Sat, 8 Dec 2007, Matt Mackall wrote:

> On Sat, Dec 08, 2007 at 06:02:38PM +0900, Osku Salerma wrote:

>> +    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)
>
> Do we actually need this function? Can't the one user of tagtypes just
> use the dict directly?

That place would then have to remember to have a useless-looking call to
localrepo.tags() first to make sure the cache is initialized, which
doesn't seem like a good idea. And we have one place now but I'm pretty
sure we'll end up with more in the future (for example, I'd like "hg tags"
to tell me which of the tags are local...).

New patch:

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

diff -r feac5b0bf9ba -r 671665f4161d mercurial/commands.py
--- a/mercurial/commands.py	Wed Nov 28 13:58:31 2007 -0800
+++ b/mercurial/commands.py	Sun Dec 09 10:19:59 2007 +0900
@@ -2628,8 +2628,13 @@ def tag(ui, repo, name, rev_=None, **opt
          rev_ = opts['rev']
      message = opts['message']
      if opts['remove']:
-        if not name in repo.tags():
-            raise util.Abort(_('tag %s does not exist') % name)
+        tagtype = repo.gettagtype(name)
+        found = ((opts['local'] and tagtype == 'local') or
+                 (not opts['local'] and tagtype == 'global'))
+
+        if not found:
+            raise util.Abort(_('%s tag %s does not exist')
+                             % (opts['local'] and 'local' or 'global', name))
          rev_ = nullid
          if not message:
              message = _('Removed tag %s') % name
diff -r feac5b0bf9ba -r 671665f4161d mercurial/localrepo.py
--- a/mercurial/localrepo.py	Wed Nov 28 13:58:31 2007 -0800
+++ b/mercurial/localrepo.py	Sun Dec 09 10:19:59 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 671665f4161d tests/test-tags
--- a/tests/test-tags	Wed Nov 28 13:58:31 2007 -0800
+++ b/tests/test-tags	Sun Dec 09 10:19:59 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 671665f4161d tests/test-tags.out
--- a/tests/test-tags.out	Wed Nov 28 13:58:31 2007 -0800
+++ b/tests/test-tags.out	Sun Dec 09 10:19:59 2007 +0900
@@ -50,7 +50,7 @@ summary:     Removed tag bar

  tip                                5:57e1983b4a60
  % remove nonexistent tag
-abort: tag foobar does not exist
+abort: global tag foobar does not exist
  changeset:   5:57e1983b4a60
  tag:         tip
  user:        test
@@ -71,3 +71,6 @@ bar                                2:72b
  % bar should still point to rev 2
  tip                                4:40af5d225513
  bar                                2:72b852876a42
+adding foo
+abort: global tag localtag does not exist
+abort: local tag globaltag does not exist

--
Osku Salerma



More information about the Mercurial-devel mailing list