All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pete Wyckoff <pw@padd.com>
To: Luke Diamand <luke@diamand.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars
Date: Tue, 17 Jan 2012 17:39:49 -0500	[thread overview]
Message-ID: <20120117223949.GA811@padd.com> (raw)
In-Reply-To: <1326755689-3344-2-git-send-email-luke@diamand.org>

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]

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

		-- Pete

  reply	other threads:[~2012-01-17 22:39 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 [this message]
2012-01-19  9:12     ` Luke Diamand
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=20120117223949.GA811@padd.com \
    --to=pw@padd.com \
    --cc=git@vger.kernel.org \
    --cc=luke@diamand.org \
    /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.