Patchwork [4,of,5] tag: only checknewlabel for new tags

login
register
mail settings
Submitter timeless@mozdev.org
Date Jan. 10, 2016, 6:57 p.m.
Message ID <8d2ac62c7f35e860bd6f.1452452271@waste.org>
Download mbox | patch
Permalink /patch/12634/
State Accepted
Headers show

Comments

timeless@mozdev.org - Jan. 10, 2016, 6:57 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1451197020 0
#      Sun Dec 27 06:17:00 2015 +0000
# Node ID 8d2ac62c7f35e860bd6fae43d6a03dd0289739b1
# Parent  1fb445a08991a9bce47a19b02aa3752df3d3cb5e
tag: only checknewlabel for new tags
Simon King - Jan. 13, 2016, 3:20 p.m.
On Sun, Jan 10, 2016 at 6:57 PM, timeless <timeless@mozdev.org> wrote:

> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1451197020 0
> #      Sun Dec 27 06:17:00 2015 +0000
> # Node ID 8d2ac62c7f35e860bd6fae43d6a03dd0289739b1
> # Parent  1fb445a08991a9bce47a19b02aa3752df3d3cb5e
> tag: only checknewlabel for new tags
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -6713,7 +6713,6 @@
>          if len(names) != len(set(names)):
>              raise error.Abort(_('tag names must be unique'))
>          for n in names:
> -            scmutil.checknewlabel(repo, n, 'tag')
>              if not n:
>                  raise error.Abort(_('tag names cannot consist entirely of
> '
>                                     'whitespace'))
> @@ -6742,6 +6741,7 @@
>                  message = 'Removed tag %s' % ', '.join(names)
>          elif not opts.get('force'):
>              for n in names:
> +                scmutil.checknewlabel(repo, n, 'tag')
>                  if n in repo.tags():
>                      raise error.Abort(_("tag '%s' already exists "
>                                         "(use -f to force)") % n)
>

Would this make it possible to use disallowed names by passing "--force"?

Simon
timeless - Jan. 14, 2016, 4:22 p.m.
Simon King wrote:
>> @@ -6742,6 +6741,7 @@
>>                  message = 'Removed tag %s' % ', '.join(names)
>>          elif not opts.get('force'):
>>              for n in names:
>> +                scmutil.checknewlabel(repo, n, 'tag')
>>                  if n in repo.tags():
>>                      raise error.Abort(_("tag '%s' already exists "
>>                                         "(use -f to force)") % n)
>
> Would this make it possible to use disallowed names by passing "--force"?

Indeed, that's not good. There's also no test coverage for that
afaict. I'll include a test so that someone like me doesn't
accidentally relax that restriction.

Thanks for spotting it!

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -6713,7 +6713,6 @@ 
         if len(names) != len(set(names)):
             raise error.Abort(_('tag names must be unique'))
         for n in names:
-            scmutil.checknewlabel(repo, n, 'tag')
             if not n:
                 raise error.Abort(_('tag names cannot consist entirely of '
                                    'whitespace'))
@@ -6742,6 +6741,7 @@ 
                 message = 'Removed tag %s' % ', '.join(names)
         elif not opts.get('force'):
             for n in names:
+                scmutil.checknewlabel(repo, n, 'tag')
                 if n in repo.tags():
                     raise error.Abort(_("tag '%s' already exists "
                                        "(use -f to force)") % n)