From: Luke Diamand <luke@diamand.org>
Date: Thu, 19 Jan 2012 09:12:16 GMT
Subject: Re: [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars
Message-ID: <4F17DE70.2010204@diamand.org>
In-Reply-To: <20120117223949.GA811@padd.com>

On 17/01/12 22:39, Pete Wyckoff wrote:
> luke@diamand.org wrote on Mon, 16 Jan 2012 23:14 +0000:
>> Don't use shell expansion when detecting branches, as it will
>> fail if the branch name contains a shell metachar. Similarly
>> for labels.
>>
>> Add additional test for branches with shell metachars.
>
> Nice.  There will be a fixup on a command in Vitor's series,
> depending on which goes first.  He'll have a couple of
> un-listified read_pipe{,_lines} that we should treat similarly.
>
>> @@ -1758,7 +1758,7 @@ class P4Sync(Command, P4UserMap):
>>       def getLabels(self):
>>           self.labels = {}
>>
>> -        l = p4CmdList("labels %s..." % ' '.join (self.depotPaths))
>> +        l = p4CmdList(["labels", "%s..." % ' '.join (self.depotPaths)])
>>           if len(l)>  0 and not self.silent:
>>               print "Finding files belonging to labels in %s" % `self.depotPaths`
>
> I suspect the command "p4" "labels" "//depot/foo/...  //depot/bar/..."
> might confuse p4, but haven't tested.  Maybe tuck each one in its
> own argument?
>
>      ["labels"] + ["%s..." % p for p in self.depotPaths]

Yes, you're right. I'll resubmit. I suspect the previous code was 
actually broken as well as you end up with the "..." only on the last depot.

>
> What happened to your failing test?  It's fun to keep the broken
> ones around to inspire others to fix them.

I'll send that one through as well. I actually had a fix, but it needs 
to be reworked now. I'm reluctant to try to do much more on this though 
unless someone can tell me how the --detect-labels code can ever really 
work.

i.e. if you do:

  p4 submit
  git-p4 rebase --detect-labels
  p4 tag LABEL_A
  git-p4 rebase --detect-labels

At this point, LABEL_A won't show up in git until the _next_ p4 commit.

It's slowly working it's way towards the top of my todo list though!

Regards!
Luke