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
next prev parent 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).