threads / patch / 27966

patch, 4 partsgit-p4 test cleanup, commit time change

Subject: [PATCH 0/4] git-p4 test cleanup, commit time change

## tl;dr

17 messages between Jul 31, 2011 and Jul 31, 2011. Diffs are folded; open one to read it.

replies: 16people: 2as markdown or json

Pete Wyckoff· Jul 31, 2011, 00:35 UTC · lore

Here's a small patch series to clean up the git-p4 tests a bit, make them work on a 2-user eval p4d, and to fix a problem with the timestamp on the import commit.

Luke, if you can take a look and ack these, I'd appreciate the review since I had to adjust your --preserveUser tests. And if you're looking at that one, might as well look at them all. :)

I'd welcome review from anyone with interest, and will plan to submit these to Junio in a week or so.

		-- Pete

Pete Wyckoff· Jul 31, 2011, 00:38 UTC · re: Pete Wyckoff · lore

[PATCH 1/4] git-p4: use test_when_finished in tests

Cleanup nicely when tests fail. This avoids many duplicated lines in the tests, and adds cleanup in a couple of tests that did not have it. When one fails, now all the rest will not fail too.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------
 1 files changed, 49 insertions(+), 54 deletions(-)

Show changes to t/t9800-git-p4.sh +49 −54

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 33b0127..aec3ba1 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '
 	cd "$TRASH_DIRECTORY"
 '
 
+cleanup_git() {
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" &&
+	mkdir "$git"
+}
+
 test_expect_success 'basic git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git log --oneline >lines &&
-	test_line_count = 1 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 1 lines
 '
 
 test_expect_success 'git-p4 clone @all' '
 	"$GITP4" clone --dest="$git" //depot@all &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git log --oneline >lines &&
-	test_line_count = 2 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 2 lines
 '
 
 test_expect_success 'git-p4 sync uninitialized repo' '
 	test_create_repo "$git" &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
-	test_must_fail "$GITP4" sync &&
-	rm -rf "$git" && mkdir "$git"
+	test_must_fail "$GITP4" sync
 '
 
 #
@@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '
 #
 test_expect_success 'git-p4 sync new branch' '
 	test_create_repo "$git" &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test_commit head &&
 	"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
 	git log --oneline p4/depot >lines &&
-	cat lines &&
-	test_line_count = 2 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 2 lines
 '
 
 test_expect_success 'exit when p4 fails to produce marshaled output' '
 	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
 	mkdir -p "$badp4dir" &&
+	test_when_finished "rm -rf $badp4dir" &&
 	cat >"$badp4dir"/p4 <<-EOF &&
 	#!$SHELL_PATH
 	exit 1
@@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '
 	echo file-wild-at >file-wild@at &&
 	echo file-wild-percent >file-wild%percent &&
 	p4 add -f file-wild* &&
-	p4 submit -d "file wildcards" &&
-	cd "$TRASH_DIRECTORY"
+	p4 submit -d "file wildcards"
 '
 
 test_expect_success 'wildcard files git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test -f file-wild#hash &&
 	test -f file-wild\*star &&
 	test -f file-wild@at &&
-	test -f file-wild%percent &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	test -f file-wild%percent
 '
 
 test_expect_success 'clone bare' '
 	"$GITP4" clone --dest="$git" --bare //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test ! -d .git &&
 	bare=`git config --get core.bare` &&
-	test "$bare" = true &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	test "$bare" = true
 '
 
 p4_add_user() {
@@ -173,6 +173,7 @@ test_expect_success 'preserve users' '
 	p4_add_user bob Bob &&
 	p4_grant_admin alice &&
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	echo "username: a change by alice" >> file1 &&
 	echo "username: a change by bob" >> file2 &&
@@ -181,27 +182,25 @@ test_expect_success 'preserve users' '
 	git config git-p4.skipSubmitEditCheck true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
 	p4_check_commit_author file1 alice &&
-	p4_check_commit_author file2 bob &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	p4_check_commit_author file2 bob
 '
 
 # Test username support, submitting as bob, who lacks admin rights. Should
 # not submit change to p4 (git diff should show deltas).
 test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	echo "username-noperms: a change by alice" >> file1 &&
 	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
 	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	! git diff --exit-code HEAD..p4/master > /dev/null &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	! git diff --exit-code HEAD..p4/master > /dev/null
 '
 
 # What happens with unknown author? Without allowMissingP4Users it should fail.
 test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git config git-p4.skipSubmitEditCheck true
 	echo "username-bob: a change by bob" >> file1 &&
@@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '
 	git config git-p4.preserveUser true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
 	git diff --exit-code HEAD..p4/master > /dev/null &&
-	p4_check_commit_author file1 alice &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	p4_check_commit_author file1 alice
 '
 
 # If we're *not* using --preserve-user, git-p4 should warn if we're submitting
@@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '
 # Test: warning disabled and user is the same.
 test_expect_success 'not preserving user with mixed authorship' '
 	"$GITP4" clone --dest="$git" //depot &&
-	(
-		cd "$git" &&
-		git config git-p4.skipSubmitEditCheck true &&
-		p4_add_user derek Derek &&
-
-		make_change_by_user usernamefile3 Derek derek@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		grep "git author derek@localhost does not match" actual &&
-
-		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		grep "git author charlie@localhost does not match" actual &&
-
-		make_change_by_user usernamefile3 alice alice@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		! grep "git author.*does not match" actual &&
-
-		git config git-p4.skipUserNameCheck true &&
-		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		! grep "git author.*does not match" actual &&
-
-		p4_check_commit_author usernamefile3 alice
-	) &&
-	rm -rf "$git" && mkdir "$git"
+	test_when_finished cleanup_git &&
+	cd "$git" &&
+	git config git-p4.skipSubmitEditCheck true &&
+	p4_add_user derek Derek &&
+
+	make_change_by_user usernamefile3 Derek derek@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	grep "git author derek@localhost does not match" actual &&
+
+	make_change_by_user usernamefile3 Charlie charlie@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	grep "git author charlie@localhost does not match" actual &&
+
+	make_change_by_user usernamefile3 alice alice@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	! grep "git author.*does not match" actual &&
+
+	git config git-p4.skipUserNameCheck true &&
+	make_change_by_user usernamefile3 Charlie charlie@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	! grep "git author.*does not match" actual &&
+
+	p4_check_commit_author usernamefile3 alice
 '

-- 
1.7.5.4

Luke Diamand· Jul 31, 2011, 09:59 UTC · re: Pete Wyckoff · lore

Re: [PATCH 1/4] git-p4: use test_when_finished in tests

On 31/07/11 01:38, Pete Wyckoff wrote:

> Cleanup nicely when tests fail.  This avoids many duplicated
> lines in the tests, and adds cleanup in a couple of tests that
> did not have it.  When one fails, now all the rest will not
> fail too.

Acked.

Much tidier.

Show 216 quoted lines

>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------
>   1 files changed, 49 insertions(+), 54 deletions(-)
>
> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
> index 33b0127..aec3ba1 100755
> --- a/t/t9800-git-p4.sh
> +++ b/t/t9800-git-p4.sh
> @@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '
>   	cd "$TRASH_DIRECTORY"
>   '
>
> +cleanup_git() {
> +	cd "$TRASH_DIRECTORY"&&
> +	rm -rf "$git"&&
> +	mkdir "$git"
> +}
> +
>   test_expect_success 'basic git-p4 clone' '
>   	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	git log --oneline>lines&&
> -	test_line_count = 1 lines&&
> -	cd ..&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test_line_count = 1 lines
>   '
>
>   test_expect_success 'git-p4 clone @all' '
>   	"$GITP4" clone --dest="$git" //depot@all&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	git log --oneline>lines&&
> -	test_line_count = 2 lines&&
> -	cd ..&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test_line_count = 2 lines
>   '
>
>   test_expect_success 'git-p4 sync uninitialized repo' '
>   	test_create_repo "$git"&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
> -	test_must_fail "$GITP4" sync&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test_must_fail "$GITP4" sync
>   '
>
>   #
> @@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '
>   #
>   test_expect_success 'git-p4 sync new branch' '
>   	test_create_repo "$git"&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	test_commit head&&
>   	"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all&&
>   	git log --oneline p4/depot>lines&&
> -	cat lines&&
> -	test_line_count = 2 lines&&
> -	cd ..&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test_line_count = 2 lines
>   '
>
>   test_expect_success 'exit when p4 fails to produce marshaled output' '
>   	badp4dir="$TRASH_DIRECTORY/badp4dir"&&
>   	mkdir -p "$badp4dir"&&
> +	test_when_finished "rm -rf $badp4dir"&&
>   	cat>"$badp4dir"/p4<<-EOF&&
>   	#!$SHELL_PATH
>   	exit 1
> @@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '
>   	echo file-wild-at>file-wild@at&&
>   	echo file-wild-percent>file-wild%percent&&
>   	p4 add -f file-wild*&&
> -	p4 submit -d "file wildcards"&&
> -	cd "$TRASH_DIRECTORY"
> +	p4 submit -d "file wildcards"
>   '
>
>   test_expect_success 'wildcard files git-p4 clone' '
>   	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	test -f file-wild#hash&&
>   	test -f file-wild\*star&&
>   	test -f file-wild@at&&
> -	test -f file-wild%percent&&
> -	cd "$TRASH_DIRECTORY"&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test -f file-wild%percent
>   '
>
>   test_expect_success 'clone bare' '
>   	"$GITP4" clone --dest="$git" --bare //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	test ! -d .git&&
>   	bare=`git config --get core.bare`&&
> -	test "$bare" = true&&
> -	cd "$TRASH_DIRECTORY"&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test "$bare" = true
>   '
>
>   p4_add_user() {
> @@ -173,6 +173,7 @@ test_expect_success 'preserve users' '
>   	p4_add_user bob Bob&&
>   	p4_grant_admin alice&&
>   	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	echo "username: a change by alice">>  file1&&
>   	echo "username: a change by bob">>  file2&&
> @@ -181,27 +182,25 @@ test_expect_success 'preserve users' '
>   	git config git-p4.skipSubmitEditCheck true&&
>   	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user&&
>   	p4_check_commit_author file1 alice&&
> -	p4_check_commit_author file2 bob&&
> -	cd "$TRASH_DIRECTORY"&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	p4_check_commit_author file2 bob
>   '
>
>   # Test username support, submitting as bob, who lacks admin rights. Should
>   # not submit change to p4 (git diff should show deltas).
>   test_expect_success 'refuse to preserve users without perms' '
>   	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	echo "username-noperms: a change by alice">>  file1&&
>   	git commit --author "Alice<alice@localhost>" -m "perms: a change by alice" file1&&
>   	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user&&
> -	! git diff --exit-code HEAD..p4/master>  /dev/null&&
> -	cd "$TRASH_DIRECTORY"&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	! git diff --exit-code HEAD..p4/master>  /dev/null
>   '
>
>   # What happens with unknown author? Without allowMissingP4Users it should fail.
>   test_expect_success 'preserve user where author is unknown to p4' '
>   	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	git config git-p4.skipSubmitEditCheck true
>   	echo "username-bob: a change by bob">>  file1&&
> @@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '
>   	git config git-p4.preserveUser true&&
>   	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit&&
>   	git diff --exit-code HEAD..p4/master>  /dev/null&&
> -	p4_check_commit_author file1 alice&&
> -	cd "$TRASH_DIRECTORY"&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	p4_check_commit_author file1 alice
>   '
>
>   # If we're *not* using --preserve-user, git-p4 should warn if we're submitting
> @@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '
>   # Test: warning disabled and user is the same.
>   test_expect_success 'not preserving user with mixed authorship' '
>   	"$GITP4" clone --dest="$git" //depot&&
> -	(
> -		cd "$git"&&
> -		git config git-p4.skipSubmitEditCheck true&&
> -		p4_add_user derek Derek&&
> -
> -		make_change_by_user usernamefile3 Derek derek@localhost&&
> -		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> -		grep "git author derek@localhost does not match" actual&&
> -
> -		make_change_by_user usernamefile3 Charlie charlie@localhost&&
> -		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> -		grep "git author charlie@localhost does not match" actual&&
> -
> -		make_change_by_user usernamefile3 alice alice@localhost&&
> -		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> -		! grep "git author.*does not match" actual&&
> -
> -		git config git-p4.skipUserNameCheck true&&
> -		make_change_by_user usernamefile3 Charlie charlie@localhost&&
> -		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> -		! grep "git author.*does not match" actual&&
> -
> -		p4_check_commit_author usernamefile3 alice
> -	)&&
> -	rm -rf "$git"&&  mkdir "$git"
> +	test_when_finished cleanup_git&&
> +	cd "$git"&&
> +	git config git-p4.skipSubmitEditCheck true&&
> +	p4_add_user derek Derek&&
> +
> +	make_change_by_user usernamefile3 Derek derek@localhost&&
> +	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> +	grep "git author derek@localhost does not match" actual&&
> +
> +	make_change_by_user usernamefile3 Charlie charlie@localhost&&
> +	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> +	grep "git author charlie@localhost does not match" actual&&
> +
> +	make_change_by_user usernamefile3 alice alice@localhost&&
> +	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> +	! grep "git author.*does not match" actual&&
> +
> +	git config git-p4.skipUserNameCheck true&&
> +	make_change_by_user usernamefile3 Charlie charlie@localhost&&
> +	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit>actual&&
> +	! grep "git author.*does not match" actual&&
> +
> +	p4_check_commit_author usernamefile3 alice
>   '
>
>

Pete Wyckoff· Jul 31, 2011, 00:39 UTC · re: Pete Wyckoff · lore

[PATCH 2/4] git-p4: make tests work on p4d eval server

The "evaluation" license for p4d allows only two users. One of the users will be auto-created as super when generating the first commit or adding the first new user. So we cannot have both an Alice and Bob as well as super. Instead, adapt the tests to work with just a single Alice user.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4.sh |   44 ++++++++++++++++++++++++++++++--------------
 1 files changed, 30 insertions(+), 14 deletions(-)

Show changes to t/t9800-git-p4.sh +30 −14

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index aec3ba1..04d8413 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -149,6 +149,13 @@ p4_grant_admin() {
 	p4 protect -i
 }
 
+p4_ungrant_admin() {
+    name=$1
+    p4 protect -o |\
+	grep -v $name |\
+	p4 protect -i
+}
+
 p4_check_commit_author() {
     file=$1
     user=$2
@@ -167,22 +174,30 @@ make_change_by_user() {
 	git commit --author "$name <$email>" -m "a change by $name"
 }
 
-# Test username support, submitting as user 'alice'
+marshal_dump() {
+	what=$1
+	python -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
+}
+
+# Test username support, submitting as user 'alice'.  Second user will be
+# whoever is running these tests.  Cannot avoid having p4d auto-create that
+# user, and the eval version only supports two users.
 test_expect_success 'preserve users' '
+	bobuser=$(p4 -G users | marshal_dump User) &&
+	bobemail=$(p4 -G users | marshal_dump Email) &&
 	p4_add_user alice Alice &&
-	p4_add_user bob Bob &&
 	p4_grant_admin alice &&
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	cd "$git" &&
 	echo "username: a change by alice" >> file1 &&
-	echo "username: a change by bob" >> file2 &&
+	echo "username: a change by $bobuser" >> file2 &&
 	git commit --author "Alice <alice@localhost>" -m "a change by alice" file1 &&
-	git commit --author "Bob <bob@localhost>" -m "a change by bob" file2 &&
+	git commit --author "$bobuser <$bobemail>" -m "a change by $bobuser" file2 &&
 	git config git-p4.skipSubmitEditCheck true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
 	p4_check_commit_author file1 alice &&
-	p4_check_commit_author file2 bob
+	p4_check_commit_author file2 $bobuser
 '
 
 # Test username support, submitting as bob, who lacks admin rights. Should
@@ -191,9 +206,10 @@ test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	cd "$git" &&
-	echo "username-noperms: a change by alice" >> file1 &&
-	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
-	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
+	p4_ungrant_admin alice &&
+	echo "username-noperms: a change by $bobuser" >> file1 &&
+	git commit --author "$bobuser <$bobemail>" -m "perms: a change by $bobuser" file1 &&
+	! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
 	! git diff --exit-code HEAD..p4/master > /dev/null
 '
 
@@ -202,9 +218,10 @@ test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true
-	echo "username-bob: a change by bob" >> file1 &&
-	git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
+	git config git-p4.skipSubmitEditCheck true &&
+	p4_grant_admin alice &&
+	echo "username-bob: a change by $bobuser" >> file1 &&
+	git commit --author "$bobuser <$bobemail>" -m "preserve: a change by $bobuser" file1 &&
 	echo "username-unknown: a change by charlie" >> file1 &&
 	git commit --author "Charlie <charlie@localhost>" -m "preserve: a change by charlie" file1 &&
 	! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
@@ -226,11 +243,10 @@ test_expect_success 'not preserving user with mixed authorship' '
 	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git config git-p4.skipSubmitEditCheck true &&
-	p4_add_user derek Derek &&
 
-	make_change_by_user usernamefile3 Derek derek@localhost &&
+	make_change_by_user usernamefile3 $bobuser $bobemail &&
 	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-	grep "git author derek@localhost does not match" actual &&
+	grep "git author $bobemail does not match" actual &&
 
 	make_change_by_user usernamefile3 Charlie charlie@localhost &&
 	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&

-- 
1.7.5.4

Pete Wyckoff· Jul 31, 2011, 00:39 UTC · re: Pete Wyckoff · lore

[PATCH 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck

Add this missing line in one of the tests. Otherwise, on fast machines, the following git-p4 commit will complain that nobody edited the submission message.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Show changes to t/t9800-git-p4.sh +1 −0

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 04d8413..24a8b79 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -207,6 +207,7 @@ test_expect_success 'refuse to preserve users without perms' '
 	test_when_finished cleanup_git &&
 	cd "$git" &&
 	p4_ungrant_admin alice &&
+	git config git-p4.skipSubmitEditCheck true &&
 	echo "username-noperms: a change by $bobuser" >> file1 &&
 	git commit --author "$bobuser <$bobemail>" -m "perms: a change by $bobuser" file1 &&
 	! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&

-- 
1.7.5.4

Luke Diamand· Jul 31, 2011, 10:01 UTC · re: Pete Wyckoff · lore

Re: [PATCH 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck

On 31/07/11 01:39, Pete Wyckoff wrote:

> Add this missing line in one of the tests.  Otherwise, on fast
> machines, the following git-p4 commit will complain that nobody
> edited the submission message.

Thanks for fixing this.

Acked.

Show 18 quoted lines

>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   t/t9800-git-p4.sh |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
> index 04d8413..24a8b79 100755
> --- a/t/t9800-git-p4.sh
> +++ b/t/t9800-git-p4.sh
> @@ -207,6 +207,7 @@ test_expect_success 'refuse to preserve users without perms' '
>   	test_when_finished cleanup_git&&
>   	cd "$git"&&
>   	p4_ungrant_admin alice&&
> +	git config git-p4.skipSubmitEditCheck true&&
>   	echo "username-noperms: a change by $bobuser">>  file1&&
>   	git commit --author "$bobuser<$bobemail>" -m "perms: a change by $bobuser" file1&&
>   	! P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user&&

Pete Wyckoff· Jul 31, 2011, 00:39 UTC · re: Pete Wyckoff · lore

[PATCH 4/4] git-p4: commit time should be most recent p4 change time

When importing a repo, the time on the initial commit had been just "now". But this causes problems when trying to share among git-p4 repos that were created identically, although at different times. Instead, use the time in the top-most p4 change as the time for the git import commit.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |   15 ++++++++++++++-
 t/t9800-git-p4.sh          |   14 ++++++++++++++
 2 files changed, 28 insertions(+), 1 deletions(-)

Show changes to 2 files +28 −1

contrib/fast-import/git-p4, t/t9800-git-p4.sh

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 98d2aee..6b9de9e 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):
     def importHeadRevision(self, revision):
         print "Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch)
 
-        details = { "user" : "git perforce import user", "time" : int(time.time()) }
+        details = {}
+        details["user"] = "git perforce import user"
         details["desc"] = ("Initial import of %s from the state at revision %s\n"
                            % (' '.join(self.depotPaths), revision))
         details["change"] = revision
@@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):
             fileCnt = fileCnt + 1
 
         details["change"] = newestRevision
+
+        # Use time from top-most change so that all git-p4 clones of
+        # the same p4 repo have the same commit SHA1s.
+        res = p4CmdList("describe -s %d" % newestRevision)
+        newestTime = None
+        for r in res:
+            if r.has_key('time'):
+                newestTime = int(r['time'])
+        if newestTime is None:
+            die("\"describe -s\" on newest change %d did not give a time")
+        details["time"] = newestTime
+
         self.updateOptionDict(details)
         try:
             self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 24a8b79..1b5f4b2 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -265,6 +265,20 @@ test_expect_success 'not preserving user with mixed authorship' '
 	p4_check_commit_author usernamefile3 alice
 '
 
+# Sleep a bit so that the top-most p4 change did not happen "now".  Then
+# import the repo and make sure that the initial import has the same time
+# as the top-most change.
+test_expect_success 'initial import time from top change time' '
+	p4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change) &&
+	p4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time) &&
+	sleep 3 &&
+	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	cd "$git" &&
+	gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
+	echo $p4time $gittime &&
+	test $p4time = $gittime
+'
 
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&

-- 
1.7.5.4

Luke Diamand· Jul 31, 2011, 10:14 UTC · re: Pete Wyckoff · lore

Re: [PATCH 4/4] git-p4: commit time should be most recent p4 change time

On 31/07/11 01:39, Pete Wyckoff wrote:

Show 5 quoted lines

> When importing a repo, the time on the initial commit had been
> just "now".  But this causes problems when trying to share among
> git-p4 repos that were created identically, although at different
> times.  Instead, use the time in the top-most p4 change as the
> time for the git import commit.

Ack.

Show 65 quoted lines

>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   contrib/fast-import/git-p4 |   15 ++++++++++++++-
>   t/t9800-git-p4.sh          |   14 ++++++++++++++
>   2 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 98d2aee..6b9de9e 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):
>       def importHeadRevision(self, revision):
>           print "Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch)
>
> -        details = { "user" : "git perforce import user", "time" : int(time.time()) }
> +        details = {}
> +        details["user"] = "git perforce import user"
>           details["desc"] = ("Initial import of %s from the state at revision %s\n"
>                              % (' '.join(self.depotPaths), revision))
>           details["change"] = revision
> @@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):
>               fileCnt = fileCnt + 1
>
>           details["change"] = newestRevision
> +
> +        # Use time from top-most change so that all git-p4 clones of
> +        # the same p4 repo have the same commit SHA1s.
> +        res = p4CmdList("describe -s %d" % newestRevision)
> +        newestTime = None
> +        for r in res:
> +            if r.has_key('time'):
> +                newestTime = int(r['time'])
> +        if newestTime is None:
> +            die("\"describe -s\" on newest change %d did not give a time")
> +        details["time"] = newestTime
> +
>           self.updateOptionDict(details)
>           try:
>               self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)
> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
> index 24a8b79..1b5f4b2 100755
> --- a/t/t9800-git-p4.sh
> +++ b/t/t9800-git-p4.sh
> @@ -265,6 +265,20 @@ test_expect_success 'not preserving user with mixed authorship' '
>   	p4_check_commit_author usernamefile3 alice
>   '
>
> +# Sleep a bit so that the top-most p4 change did not happen "now".  Then
> +# import the repo and make sure that the initial import has the same time
> +# as the top-most change.
> +test_expect_success 'initial import time from top change time' '
> +	p4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change)&&
> +	p4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time)&&
> +	sleep 3&&
> +	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
> +	cd "$git"&&
> +	gittime=$(git show -s --raw --pretty=format:%at HEAD)&&
> +	echo $p4time $gittime&&
> +	test $p4time = $gittime
> +'
>
>   test_expect_success 'shutdown' '
>   	pid=`pgrep -f p4d`&&

Luke Diamand· Jul 31, 2011, 09:49 UTC · re: Pete Wyckoff · lore

Re: [PATCH 0/4] git-p4 test cleanup, commit time change

On 31/07/11 01:35, Pete Wyckoff wrote:

Show 11 quoted lines

> Here's a small patch series to clean up the git-p4 tests a bit,
> make them work on a 2-user eval p4d, and to fix a problem with
> the timestamp on the import commit.
>
> Luke, if you can take a look and ack these, I'd appreciate
> the review since I had to adjust your --preserveUser tests.
> And if you're looking at that one, might as well look at them
> all.  :)
>
> I'd welcome review from anyone with interest, and will plan
> to submit these to Junio in a week or so.

Just looking at them now.

One thing though - are you sure the p4d demo license only allows 2 users? I've only ever used the demo p4d for testing and it seems to work fine (just tried again with version P4D/LINUX26X86_64/2010.2/322263).

The web page says this:

"The free Perforce Server supports two users and five client workspaces, or unlimited users and up to 1,000 files. Request a 45-day evaluation license to support any number of users and unlimited files."

Which I assume means the test harness couldn't have more than 1000 files and some unspecified number of client workspaces?

I've just created 4 users by hand with no obvious problems. I'm pretty sure I'm not accidentally using the corporate server!

Confuzzled!

Pete Wyckoff· Jul 31, 2011, 13:42 UTC · re: Luke Diamand · lore

Re: [PATCH 0/4] git-p4 test cleanup, commit time change

luke@diamand.org wrote on Sun, 31 Jul 2011 10:49 +0100:

> One thing though - are you sure the p4d demo license only allows 2
> users? I've only ever used the demo p4d for testing and it seems to
> work fine (just tried again with version
> P4D/LINUX26X86_64/2010.2/322263).

You are so right. I was using an older p4d (2009.1/222893) at home, and didn't even think about checking for updates.

There was one line in patch 3 that should survive, just to add a "&&". I've added your acks for the rest and will repost to show the current changes.

Thanks,

		-- Pete

Luke Diamand· Jul 31, 2011, 16:01 UTC · re: Pete Wyckoff · lore

Re: [PATCH 0/4] git-p4 test cleanup, commit time change

On 31/07/11 14:42, Pete Wyckoff wrote:

Show 12 quoted lines

> luke@diamand.org wrote on Sun, 31 Jul 2011 10:49 +0100:
>> One thing though - are you sure the p4d demo license only allows 2
>> users? I've only ever used the demo p4d for testing and it seems to
>> work fine (just tried again with version
>> P4D/LINUX26X86_64/2010.2/322263).
>
> You are so right.  I was using an older p4d (2009.1/222893) at
> home, and didn't even think about checking for updates.
>
> There was one line in patch 3 that should survive, just to add a
> "&&".  I've added your acks for the rest and will repost to show
> the current changes.

All looks good to me!

Luke

Pete Wyckoff· Jul 31, 2011, 13:44 UTC · re: Pete Wyckoff · lore

[PATCHv2 0/4] git-p4 test cleanup, commit time change

Re-spun, removing all of the original patch 2 now that Luke pointed out that a new p4d is available.

Original intro:

    Here's a small patch series to clean up the git-p4 tests a bit,
    make them work on a 2-user eval p4d, and to fix a problem with
    the timestamp on the import commit.

Pete Wyckoff· Jul 31, 2011, 13:44 UTC · re: Pete Wyckoff · lore

[PATCHv2 1/4] git-p4: use test_when_finished in tests

Cleanup nicely when tests fail. This avoids many duplicated lines in the tests, and adds cleanup in a couple of tests that did not have it. When one fails, now all the rest will not fail too.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Acked-by: Luke Diamand <luke@diamand.org>
---
 t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------
 1 files changed, 49 insertions(+), 54 deletions(-)

Show changes to t/t9800-git-p4.sh +49 −54

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 33b0127..aec3ba1 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '
 	cd "$TRASH_DIRECTORY"
 '
 
+cleanup_git() {
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" &&
+	mkdir "$git"
+}
+
 test_expect_success 'basic git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git log --oneline >lines &&
-	test_line_count = 1 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 1 lines
 '
 
 test_expect_success 'git-p4 clone @all' '
 	"$GITP4" clone --dest="$git" //depot@all &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git log --oneline >lines &&
-	test_line_count = 2 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 2 lines
 '
 
 test_expect_success 'git-p4 sync uninitialized repo' '
 	test_create_repo "$git" &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
-	test_must_fail "$GITP4" sync &&
-	rm -rf "$git" && mkdir "$git"
+	test_must_fail "$GITP4" sync
 '
 
 #
@@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '
 #
 test_expect_success 'git-p4 sync new branch' '
 	test_create_repo "$git" &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test_commit head &&
 	"$GITP4" sync --branch=refs/remotes/p4/depot //depot@all &&
 	git log --oneline p4/depot >lines &&
-	cat lines &&
-	test_line_count = 2 lines &&
-	cd .. &&
-	rm -rf "$git" && mkdir "$git"
+	test_line_count = 2 lines
 '
 
 test_expect_success 'exit when p4 fails to produce marshaled output' '
 	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
 	mkdir -p "$badp4dir" &&
+	test_when_finished "rm -rf $badp4dir" &&
 	cat >"$badp4dir"/p4 <<-EOF &&
 	#!$SHELL_PATH
 	exit 1
@@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '
 	echo file-wild-at >file-wild@at &&
 	echo file-wild-percent >file-wild%percent &&
 	p4 add -f file-wild* &&
-	p4 submit -d "file wildcards" &&
-	cd "$TRASH_DIRECTORY"
+	p4 submit -d "file wildcards"
 '
 
 test_expect_success 'wildcard files git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test -f file-wild#hash &&
 	test -f file-wild\*star &&
 	test -f file-wild@at &&
-	test -f file-wild%percent &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	test -f file-wild%percent
 '
 
 test_expect_success 'clone bare' '
 	"$GITP4" clone --dest="$git" --bare //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	test ! -d .git &&
 	bare=`git config --get core.bare` &&
-	test "$bare" = true &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	test "$bare" = true
 '
 
 p4_add_user() {
@@ -173,6 +173,7 @@ test_expect_success 'preserve users' '
 	p4_add_user bob Bob &&
 	p4_grant_admin alice &&
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	echo "username: a change by alice" >> file1 &&
 	echo "username: a change by bob" >> file2 &&
@@ -181,27 +182,25 @@ test_expect_success 'preserve users' '
 	git config git-p4.skipSubmitEditCheck true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit --preserve-user &&
 	p4_check_commit_author file1 alice &&
-	p4_check_commit_author file2 bob &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	p4_check_commit_author file2 bob
 '
 
 # Test username support, submitting as bob, who lacks admin rights. Should
 # not submit change to p4 (git diff should show deltas).
 test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	echo "username-noperms: a change by alice" >> file1 &&
 	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
 	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&
-	! git diff --exit-code HEAD..p4/master > /dev/null &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	! git diff --exit-code HEAD..p4/master > /dev/null
 '
 
 # What happens with unknown author? Without allowMissingP4Users it should fail.
 test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
 	cd "$git" &&
 	git config git-p4.skipSubmitEditCheck true
 	echo "username-bob: a change by bob" >> file1 &&
@@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '
 	git config git-p4.preserveUser true &&
 	P4EDITOR=touch P4USER=alice P4PASSWD=secret "$GITP4" commit &&
 	git diff --exit-code HEAD..p4/master > /dev/null &&
-	p4_check_commit_author file1 alice &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	p4_check_commit_author file1 alice
 '
 
 # If we're *not* using --preserve-user, git-p4 should warn if we're submitting
@@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '
 # Test: warning disabled and user is the same.
 test_expect_success 'not preserving user with mixed authorship' '
 	"$GITP4" clone --dest="$git" //depot &&
-	(
-		cd "$git" &&
-		git config git-p4.skipSubmitEditCheck true &&
-		p4_add_user derek Derek &&
-
-		make_change_by_user usernamefile3 Derek derek@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		grep "git author derek@localhost does not match" actual &&
-
-		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		grep "git author charlie@localhost does not match" actual &&
-
-		make_change_by_user usernamefile3 alice alice@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		! grep "git author.*does not match" actual &&
-
-		git config git-p4.skipUserNameCheck true &&
-		make_change_by_user usernamefile3 Charlie charlie@localhost &&
-		P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
-		! grep "git author.*does not match" actual &&
-
-		p4_check_commit_author usernamefile3 alice
-	) &&
-	rm -rf "$git" && mkdir "$git"
+	test_when_finished cleanup_git &&
+	cd "$git" &&
+	git config git-p4.skipSubmitEditCheck true &&
+	p4_add_user derek Derek &&
+
+	make_change_by_user usernamefile3 Derek derek@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	grep "git author derek@localhost does not match" actual &&
+
+	make_change_by_user usernamefile3 Charlie charlie@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	grep "git author charlie@localhost does not match" actual &&
+
+	make_change_by_user usernamefile3 alice alice@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	! grep "git author.*does not match" actual &&
+
+	git config git-p4.skipUserNameCheck true &&
+	make_change_by_user usernamefile3 Charlie charlie@localhost &&
+	P4EDITOR=cat P4USER=alice P4PASSWD=secret "$GITP4" commit >actual &&
+	! grep "git author.*does not match" actual &&
+
+	p4_check_commit_author usernamefile3 alice
 '

-- 
1.7.5.4

Pete Wyckoff· Jul 31, 2011, 13:45 UTC · re: Pete Wyckoff · lore

[PATCHv2 2/4] git-p4: add missing && in test

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to t/t9800-git-p4.sh +1 −1

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index aec3ba1..b7eda82 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -202,7 +202,7 @@ test_expect_success 'preserve user where author is unknown to p4' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	cd "$git" &&
-	git config git-p4.skipSubmitEditCheck true
+	git config git-p4.skipSubmitEditCheck true &&
 	echo "username-bob: a change by bob" >> file1 &&
 	git commit --author "Bob <bob@localhost>" -m "preserve: a change by bob" file1 &&
 	echo "username-unknown: a change by charlie" >> file1 &&

-- 
1.7.5.4

Luke Diamand· Jul 31, 2011, 13:55 UTC · re: Pete Wyckoff · lore

Re: [PATCHv2 2/4] git-p4: add missing && in test

Acked.

On 31/07/11 14:45, Pete Wyckoff wrote:

Show 18 quoted lines

> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   t/t9800-git-p4.sh |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
> index aec3ba1..b7eda82 100755
> --- a/t/t9800-git-p4.sh
> +++ b/t/t9800-git-p4.sh
> @@ -202,7 +202,7 @@ test_expect_success 'preserve user where author is unknown to p4' '
>   	"$GITP4" clone --dest="$git" //depot&&
>   	test_when_finished cleanup_git&&
>   	cd "$git"&&
> -	git config git-p4.skipSubmitEditCheck true
> +	git config git-p4.skipSubmitEditCheck true&&
>   	echo "username-bob: a change by bob">>  file1&&
>   	git commit --author "Bob<bob@localhost>" -m "preserve: a change by bob" file1&&
>   	echo "username-unknown: a change by charlie">>  file1&&

Pete Wyckoff· Jul 31, 2011, 13:45 UTC · re: Pete Wyckoff · lore

[PATCHv2 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck

Add this missing line in one of the tests. Otherwise, on fast machines, the following git-p4 commit will complain that nobody edited the submission message.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Acked-by: Luke Diamand <luke@diamand.org>
---
 t/t9800-git-p4.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Show changes to t/t9800-git-p4.sh +1 −0

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index b7eda82..b304707 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -191,6 +191,7 @@ test_expect_success 'refuse to preserve users without perms' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	cd "$git" &&
+	git config git-p4.skipSubmitEditCheck true &&
 	echo "username-noperms: a change by alice" >> file1 &&
 	git commit --author "Alice <alice@localhost>" -m "perms: a change by alice" file1 &&
 	! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user &&

-- 
1.7.5.4

Pete Wyckoff· Jul 31, 2011, 13:45 UTC · re: Pete Wyckoff · lore

[PATCHv2 4/4] git-p4: commit time should be most recent p4 change time

Signed-off-by: Pete Wyckoff <pw@padd.com>
Acked-by: Luke Diamand <luke@diamand.org>
---
 contrib/fast-import/git-p4 |   15 ++++++++++++++-
 t/t9800-git-p4.sh          |   19 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletions(-)

Show changes to 2 files +33 −1

contrib/fast-import/git-p4, t/t9800-git-p4.sh

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 98d2aee..6b9de9e 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):
     def importHeadRevision(self, revision):
         print "Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch)
 
-        details = { "user" : "git perforce import user", "time" : int(time.time()) }
+        details = {}
+        details["user"] = "git perforce import user"
         details["desc"] = ("Initial import of %s from the state at revision %s\n"
                            % (' '.join(self.depotPaths), revision))
         details["change"] = revision
@@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):
             fileCnt = fileCnt + 1
 
         details["change"] = newestRevision
+
+        # Use time from top-most change so that all git-p4 clones of
+        # the same p4 repo have the same commit SHA1s.
+        res = p4CmdList("describe -s %d" % newestRevision)
+        newestTime = None
+        for r in res:
+            if r.has_key('time'):
+                newestTime = int(r['time'])
+        if newestTime is None:
+            die("\"describe -s\" on newest change %d did not give a time")
+        details["time"] = newestTime
+
         self.updateOptionDict(details)
         try:
             self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index b304707..97ec975 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -249,6 +249,25 @@ test_expect_success 'not preserving user with mixed authorship' '
 	p4_check_commit_author usernamefile3 alice
 '
 
+marshal_dump() {
+	what=$1
+	python -c 'import marshal, sys; d = marshal.load(sys.stdin); print d["'$what'"]'
+}
+
+# Sleep a bit so that the top-most p4 change did not happen "now".  Then
+# import the repo and make sure that the initial import has the same time
+# as the top-most change.
+test_expect_success 'initial import time from top change time' '
+	p4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change) &&
+	p4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time) &&
+	sleep 3 &&
+	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	cd "$git" &&
+	gittime=$(git show -s --raw --pretty=format:%at HEAD) &&
+	echo $p4time $gittime &&
+	test $p4time = $gittime
+'
 
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&

-- 
1.7.5.4