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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.