public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Pete Wyckoff <pw@padd•com>
To: Luke Diamand <luke@diamand•org>
Cc: git@vger•kernel.org, vitor.hda@gmail•com
Subject: Re: [RFC/PATCHv2] git-p4: handle files with shell metacharacters
Date: Tue, 27 Sep 2011 09:03:34 -0400	[thread overview]
Message-ID: <20110927130334.GA24327@arf.padd.com> (raw)
In-Reply-To: <1317112836-14135-1-git-send-email-luke@diamand.org>

luke@diamand•org wrote on Tue, 27 Sep 2011 09:40 +0100:
> Updated git-p4 changes incorporating Pete's comments.
> 
>  - p4CmdList's stdin argument can now be a list.

I think this fits in with the rest of the patch and can stay.

>  - Getting rid of the string option entirely is very hard; there are
>    places where currently git-p4 creates a pipeline.

Yeah, thanks for checking though.  Best to leave it consistent
like you did.

>  - I wonder if verbose should actually be enabled for all the test
>    cases?

It is way too verbose, even for test, but I see the argument.
One easy place to change it would be in the definition in
t/lib-git-p4.sh.  You could do this by hand when testing the
tests perhaps.

>  - The $ENV{PWD} is needed now because the shell used to set that; now
>    that the shell isn't in use git-p4 has to set it.
> 
> Pete - I wasn't sure whether you were saying I should rework
> my patch against next (and you would then rework your series) or
> something else. That sounds complicated though - let me know!

If you don't mind, I'll just queue it up with the utf16 and
test-refactor stuff I have, and send it all to Junio post-1.7.7.
Here's how I plan to adjust your tests, given the feedback that
Junio gave earlier and from reading other tests in t/.

		-- Pete


-----------8<------------------
From 6b4bd671df338210ffd0348358420f0feb6f35c0 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd•com>
Date: Tue, 27 Sep 2011 08:53:25 -0400
Subject: [PATCH] git-p4 t9803: align syntax with other tests


Signed-off-by: Pete Wyckoff <pw@padd•com>
---
 t/t9803-git-shell-metachars.sh |   30 ++++++++++++------------------
 1 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/t/t9803-git-shell-metachars.sh b/t/t9803-git-shell-metachars.sh
index c166603..6cf4298 100755
--- a/t/t9803-git-shell-metachars.sh
+++ b/t/t9803-git-shell-metachars.sh
@@ -5,9 +5,7 @@ test_description='git-p4 transparency to shell metachars in filenames'
 . ./lib-git-p4.sh
 
 test_expect_success 'start p4d' '
-	kill_p4d || : &&
-	start_p4d &&
-	cd "$TRASH_DIRECTORY"
+	start_p4d
 '
 
 test_expect_success 'init depot' '
@@ -30,25 +28,18 @@ test_expect_success 'shell metachars in filenames' '
 		echo f2 >"file with spaces" &&
 		git add "file with spaces" &&
 		P4EDITOR=touch git commit -m "add files" &&
-		"$GITP4" submit --verbose &&
+		"$GITP4" submit
+	) &&
+	(
 		cd "$cli" &&
 		p4 sync ... &&
-		ls -l "file with spaces" &&
-		ls -l "foo\$bar"
+		test -e "file with spaces" &&
+		test -e "foo\$bar"
 	)
 '
 
-check_missing () {
-	for i in $*; do
-		if [ -f $i ]; then
-			echo $i found but should be missing 1>&2
-			exit 1
-		fi
-	done
-}
-
 test_expect_success 'deleting with shell metachars' '
-	"$GITP4" clone --dest="$git" --verbose //depot &&
+	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
@@ -56,10 +47,13 @@ test_expect_success 'deleting with shell metachars' '
 		git rm foo\$bar &&
 		git rm file\ with\ spaces &&
 		P4EDITOR=touch git commit -m "remove files" &&
-		"$GITP4" submit --verbose
+		"$GITP4" submit
+	) &&
+	(
 		cd "$cli" &&
 		p4 sync ... &&
-		check_missing "file with spaces" foo\$bar
+		test ! -e "file with spaces" &&
+		test ! -e foo\$bar
 	)
 '
 
-- 
1.7.6.3

  parent reply	other threads:[~2011-09-27 13:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-26 21:29 [PATCH/RFCv1] git-p4: handle files with shell metacharacters Luke Diamand
2011-09-26 21:29 ` Luke Diamand
2011-09-26 21:47   ` Pete Wyckoff
2011-09-27  8:40     ` [RFC/PATCHv2] " Luke Diamand
2011-09-27  8:40       ` Luke Diamand
2011-09-27  9:32       ` Luke Diamand
2011-09-27 13:03       ` Pete Wyckoff [this message]
2011-09-30  9:06         ` 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=20110927130334.GA24327@arf.padd.com \
    --to=pw@padd$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=luke@diamand$(echo .)org \
    --cc=vitor.hda@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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