Patchwork [stable] convert: convert real global tags only - not local tags

login
register
mail settings
Submitter Mads Kiilerich
Date May 14, 2014, 11:50 a.m.
Message ID <6878d40f8a2153885374.1400068203@mk-desktop>
Download mbox | patch
Permalink /patch/4737/
State Superseded
Headers show

Comments

Mads Kiilerich - May 14, 2014, 11:50 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1400068180 -7200
#      Wed May 14 13:49:40 2014 +0200
# Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
# Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
convert: convert real global tags only - not local tags
Pierre-Yves David - May 15, 2014, 6:28 a.m.
On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1400068180 -7200
> #      Wed May 14 13:49:40 2014 +0200
> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
> convert: convert real global tags only - not local tags


Not sure to understand what this patch is about.

What is the problematic behavior before this patches. How does this 
patches fixes it?
Sean Farley - May 15, 2014, 7:06 p.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1400068180 -7200
>> #      Wed May 14 13:49:40 2014 +0200
>> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
>> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
>> convert: convert real global tags only - not local tags
>
>
> Not sure to understand what this patch is about.
>
> What is the problematic behavior before this patches. How does this 
> patches fixes it?

If hg-git or hgremotebranches is enabled, then there are a lot of local
tags set that 'hg convert' would then copy over. That is definitely a
flaw.
Angel Ezquerra - May 15, 2014, 7:11 p.m.
El 15/05/2014 21:06, "Sean Farley" <sean.michael.farley@gmail.com> escribió:
>
>
> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>
> > On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
> >> # HG changeset patch
> >> # User Mads Kiilerich <madski@unity3d.com>
> >> # Date 1400068180 -7200
> >> #      Wed May 14 13:49:40 2014 +0200
> >> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
> >> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
> >> convert: convert real global tags only - not local tags
> >
> >
> > Not sure to understand what this patch is about.
> >
> > What is the problematic behavior before this patches. How does this
> > patches fixes it?
>
> If hg-git or hgremotebranches is enabled, then there are a lot of local
> tags set that 'hg convert' would then copy over. That is definitely a
> flaw.

I agree.

BTW I recently discovered the hgremotebranches extension and I love it.
Props to Augie for making yet another awesome extension!

Angel
Pierre-Yves David - May 15, 2014, 7:16 p.m.
On 05/15/2014 12:06 PM, Sean Farley wrote:
>
> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>
>> On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1400068180 -7200
>>> #      Wed May 14 13:49:40 2014 +0200
>>> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
>>> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
>>> convert: convert real global tags only - not local tags
>>
>>
>> Not sure to understand what this patch is about.
>>
>> What is the problematic behavior before this patches. How does this
>> patches fixes it?
>
> If hg-git or hgremotebranches is enabled, then there are a lot of local
> tags set that 'hg convert' would then copy over. That is definitely a
> flaw.

Sounds requires a description udpate -and- a comment in the code.
Mads Kiilerich - May 16, 2014, 12:32 a.m.
On 05/15/2014 09:16 PM, Pierre-Yves David wrote:
>
>
> On 05/15/2014 12:06 PM, Sean Farley wrote:
>>
>> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>>
>>> On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
>>>> # HG changeset patch
>>>> # User Mads Kiilerich <madski@unity3d.com>
>>>> # Date 1400068180 -7200
>>>> #      Wed May 14 13:49:40 2014 +0200
>>>> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
>>>> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
>>>> convert: convert real global tags only - not local tags
>>>
>>>
>>> Not sure to understand what this patch is about.

This is about convert and tags, local and global.

>>>
>>> What is the problematic behavior before this patches. 

Convert did not only convert global tags - it also converted local tags.

>>> How does this patches fixes it?

It makes it so that convert only converts global tags - not local tags.

> Sounds requires a description udpate -and- a comment in the code.

It is ridiculous to have to describe the basic functionality of a sub 
system every time a patch touches it.

What comment would you add? "we only consider the tags from taglist 
where the type is global"?

/Mads
Pierre-Yves David - May 16, 2014, 12:46 a.m.
On 05/15/2014 05:32 PM, Mads Kiilerich wrote:
> On 05/15/2014 09:16 PM, Pierre-Yves David wrote:
>>
>>
>> On 05/15/2014 12:06 PM, Sean Farley wrote:
>>>
>>> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>>>
>>>> On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
>>>>> # HG changeset patch
>>>>> # User Mads Kiilerich <madski@unity3d.com>
>>>>> # Date 1400068180 -7200
>>>>> #      Wed May 14 13:49:40 2014 +0200
>>>>> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
>>>>> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
>>>>> convert: convert real global tags only - not local tags
>>>>
>>>>
>>>> Not sure to understand what this patch is about.
>
> This is about convert and tags, local and global.
>
>>>>
>>>> What is the problematic behavior before this patches.
>
> Convert did not only convert global tags - it also converted local tags.

But why is converting local tag bad? Sound perfectly reasonable things 
to do from here.


>> Sounds requires a description udpate -and- a comment in the code.
>
> It is ridiculous to have to describe the basic functionality of a sub
> system every time a patch touches it.

It is ridiculous to receives patches for review that introduces 
significant behavior changes without any justifications.

We cannot both encourage everyone to do some review and send them 
cryptic patches that requires paying attention to 3 week of prior 
discussion (on another channel?).

I agree we cannot explain every small details. But your patch should be 
processable by a decently wide audience. Including the guy that will use 
annotate in 6 months to figure out what is going on.

Also if you do not add a comment in the code about why we are converting 
global tags only. In 3 months we'll receive the opposite  changeset that 
enables conversion of local tags.


> What comment would you add? "we only consider the tags from taglist
> where the type is global"?

I do not want just a useless re-phrasing. You are suggesting an update 
that, to me, look like

   paint car in red

   Apply red paint on car.

I would like something that looks like

   paint car in red

   Car used to be black, a color reserved for secret service and bad
   guys usage. We re-paint it in red as red car are known to be faster
   anyway.

So please update this description and comment your code.

Patch

diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -394,7 +394,8 @@  class mercurial_source(converter_source)
                       sortkey=ctx.rev())
 
     def gettags(self):
-        tags = [t for t in self.repo.tagslist() if t[0] != 'tip']
+        tags = [t for t in self.repo.tagslist()
+                if t[0] != 'tip' and self.repo.tagtype(t[0]) == 'global']
         return dict([(name, hex(node)) for name, node in tags
                      if self.keep(node)])
 
diff --git a/tests/test-convert-hg-sink.t b/tests/test-convert-hg-sink.t
--- a/tests/test-convert-hg-sink.t
+++ b/tests/test-convert-hg-sink.t
@@ -16,8 +16,10 @@ 
   $ echo file > foo/file
   $ hg ci -qAm 'add foo/file'
   $ hg tag some-tag
+  $ hg tag -l local-tag
   $ hg log
   changeset:   3:593cbf6fb2b4
+  tag:         local-tag
   tag:         tip
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000