git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luke Diamand <luke@diamand.org>
To: Pete Wyckoff <pw@padd.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars
Date: Thu, 19 Jan 2012 09:12:16 +0000	[thread overview]
Message-ID: <4F17DE70.2010204@diamand.org> (raw)
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

  reply	other threads:[~2012-01-19  9:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-16 23:14 [PATCHv3 0/4] git-p4: small fixes to branches and labels Luke Diamand
2012-01-16 23:14 ` [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars Luke Diamand
2012-01-17 22:39   ` Pete Wyckoff
2012-01-19  9:12     ` Luke Diamand [this message]
2012-01-16 23:14 ` [PATCH 2/4] git-p4: cope with labels with empty descriptions Luke Diamand
2012-01-16 23:14 ` [PATCH 3/4] git-p4: importing labels should cope with missing owner Luke Diamand
2012-01-16 23:14 ` [PATCH 4/4] git-p4: add test for p4 labels Luke Diamand
  -- strict thread matches above, loose matches on Subject: below --
2011-11-07 21:36 [RFC/PATCH 0/4] git-p4: small fixes to branches and labels; tests Luke Diamand
2011-11-07 21:36 ` [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars Luke Diamand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F17DE70.2010204@diamand.org \
    --to=luke@diamand.org \
    --cc=git@vger.kernel.org \
    --cc=pw@padd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).