{"thread":{"id":"27966","subject":"[PATCH 0/4] git-p4 test cleanup, commit time change","startedAt":"2011-07-31T00:35:57Z","lastAt":"2011-07-31T16:01:12Z","messageCount":17,"participants":["Pete Wyckoff","Luke Diamand"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"172414","messageId":"20110731003557.GA4867@arf.padd.com","threadId":"27966","inReplyTo":null,"subject":"[PATCH 0/4] git-p4 test cleanup, commit time change","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T00:35:57Z","receivedAt":"2011-07-31T00:35:57Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Here's a small patch series to clean up the git-p4 tests a bit,\nmake them work on a 2-user eval p4d, and to fix a problem with\nthe timestamp on the import commit.\n\nLuke, if you can take a look and ack these, I'd appreciate\nthe review since I had to adjust your --preserveUser tests.\nAnd if you're looking at that one, might as well look at them\nall.  :)\n\nI'd welcome review from anyone with interest, and will plan\nto submit these to Junio in a week or so.\n\n\t\t-- Pete\n"},{"id":"172416","messageId":"20110731003855.GB4867@arf.padd.com","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"[PATCH 1/4] git-p4: use test_when_finished in tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T00:38:55Z","receivedAt":"2011-07-31T00:38:55Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Cleanup nicely when tests fail.  This avoids many duplicated\nlines in the tests, and adds cleanup in a couple of tests that\ndid not have it.  When one fails, now all the rest will not\nfail too.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------\n 1 files changed, 49 insertions(+), 54 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 33b0127..aec3ba1 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '\n \tcd \"$TRASH_DIRECTORY\"\n '\n \n+cleanup_git() {\n+\tcd \"$TRASH_DIRECTORY\" &&\n+\trm -rf \"$git\" &&\n+\tmkdir \"$git\"\n+}\n+\n test_expect_success 'basic git-p4 clone' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit log --oneline >lines &&\n-\ttest_line_count = 1 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 1 lines\n '\n \n test_expect_success 'git-p4 clone @all' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit log --oneline >lines &&\n-\ttest_line_count = 2 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 2 lines\n '\n \n test_expect_success 'git-p4 sync uninitialized repo' '\n \ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n-\ttest_must_fail \"$GITP4\" sync &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_must_fail \"$GITP4\" sync\n '\n \n #\n@@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '\n #\n test_expect_success 'git-p4 sync new branch' '\n \ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest_commit head &&\n \t\"$GITP4\" sync --branch=refs/remotes/p4/depot //depot@all &&\n \tgit log --oneline p4/depot >lines &&\n-\tcat lines &&\n-\ttest_line_count = 2 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 2 lines\n '\n \n test_expect_success 'exit when p4 fails to produce marshaled output' '\n \tbadp4dir=\"$TRASH_DIRECTORY/badp4dir\" &&\n \tmkdir -p \"$badp4dir\" &&\n+\ttest_when_finished \"rm -rf $badp4dir\" &&\n \tcat >\"$badp4dir\"/p4 <<-EOF &&\n \t#!$SHELL_PATH\n \texit 1\n@@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '\n \techo file-wild-at >file-wild@at &&\n \techo file-wild-percent >file-wild%percent &&\n \tp4 add -f file-wild* &&\n-\tp4 submit -d \"file wildcards\" &&\n-\tcd \"$TRASH_DIRECTORY\"\n+\tp4 submit -d \"file wildcards\"\n '\n \n test_expect_success 'wildcard files git-p4 clone' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest -f file-wild#hash &&\n \ttest -f file-wild\\*star &&\n \ttest -f file-wild@at &&\n-\ttest -f file-wild%percent &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest -f file-wild%percent\n '\n \n test_expect_success 'clone bare' '\n \t\"$GITP4\" clone --dest=\"$git\" --bare //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest ! -d .git &&\n \tbare=`git config --get core.bare` &&\n-\ttest \"$bare\" = true &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest \"$bare\" = true\n '\n \n p4_add_user() {\n@@ -173,6 +173,7 @@ test_expect_success 'preserve users' '\n \tp4_add_user bob Bob &&\n \tp4_grant_admin alice &&\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \techo \"username: a change by alice\" >> file1 &&\n \techo \"username: a change by bob\" >> file2 &&\n@@ -181,27 +182,25 @@ test_expect_success 'preserve users' '\n \tgit config git-p4.skipSubmitEditCheck true &&\n \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n \tp4_check_commit_author file1 alice &&\n-\tp4_check_commit_author file2 bob &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\tp4_check_commit_author file2 bob\n '\n \n # Test username support, submitting as bob, who lacks admin rights. Should\n # not submit change to p4 (git diff should show deltas).\n test_expect_success 'refuse to preserve users without perms' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \techo \"username-noperms: a change by alice\" >> file1 &&\n \tgit commit --author \"Alice <alice@localhost>\" -m \"perms: a change by alice\" file1 &&\n \t! P4EDITOR=touch P4USER=bob P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n-\t! git diff --exit-code HEAD..p4/master > /dev/null &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\t! git diff --exit-code HEAD..p4/master > /dev/null\n '\n \n # What happens with unknown author? Without allowMissingP4Users it should fail.\n test_expect_success 'preserve user where author is unknown to p4' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit config git-p4.skipSubmitEditCheck true\n \techo \"username-bob: a change by bob\" >> file1 &&\n@@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '\n \tgit config git-p4.preserveUser true &&\n \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit &&\n \tgit diff --exit-code HEAD..p4/master > /dev/null &&\n-\tp4_check_commit_author file1 alice &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\tp4_check_commit_author file1 alice\n '\n \n # If we're *not* using --preserve-user, git-p4 should warn if we're submitting\n@@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '\n # Test: warning disabled and user is the same.\n test_expect_success 'not preserving user with mixed authorship' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n-\t(\n-\t\tcd \"$git\" &&\n-\t\tgit config git-p4.skipSubmitEditCheck true &&\n-\t\tp4_add_user derek Derek &&\n-\n-\t\tmake_change_by_user usernamefile3 Derek derek@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\tgrep \"git author derek@localhost does not match\" actual &&\n-\n-\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\tgrep \"git author charlie@localhost does not match\" actual &&\n-\n-\t\tmake_change_by_user usernamefile3 alice alice@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\t! grep \"git author.*does not match\" actual &&\n-\n-\t\tgit config git-p4.skipUserNameCheck true &&\n-\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\t! grep \"git author.*does not match\" actual &&\n-\n-\t\tp4_check_commit_author usernamefile3 alice\n-\t) &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_when_finished cleanup_git &&\n+\tcd \"$git\" &&\n+\tgit config git-p4.skipSubmitEditCheck true &&\n+\tp4_add_user derek Derek &&\n+\n+\tmake_change_by_user usernamefile3 Derek derek@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\tgrep \"git author derek@localhost does not match\" actual &&\n+\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\tgrep \"git author charlie@localhost does not match\" actual &&\n+\n+\tmake_change_by_user usernamefile3 alice alice@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\t! grep \"git author.*does not match\" actual &&\n+\n+\tgit config git-p4.skipUserNameCheck true &&\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\t! grep \"git author.*does not match\" actual &&\n+\n+\tp4_check_commit_author usernamefile3 alice\n '\n \n \n-- \n1.7.5.4\n"},{"id":"172417","messageId":"20110731003913.GC4867@arf.padd.com","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"[PATCH 2/4] git-p4: make tests work on p4d eval server","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T00:39:13Z","receivedAt":"2011-07-31T00:39:13Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The \"evaluation\" license for p4d allows only two users.  One of\nthe users will be auto-created as super when generating the first\ncommit or adding the first new user.  So we cannot have both an\nAlice and Bob as well as super.  Instead, adapt the tests to work\nwith just a single Alice user.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9800-git-p4.sh |   44 ++++++++++++++++++++++++++++++--------------\n 1 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex aec3ba1..04d8413 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -149,6 +149,13 @@ p4_grant_admin() {\n \tp4 protect -i\n }\n \n+p4_ungrant_admin() {\n+    name=$1\n+    p4 protect -o |\\\n+\tgrep -v $name |\\\n+\tp4 protect -i\n+}\n+\n p4_check_commit_author() {\n     file=$1\n     user=$2\n@@ -167,22 +174,30 @@ make_change_by_user() {\n \tgit commit --author \"$name <$email>\" -m \"a change by $name\"\n }\n \n-# Test username support, submitting as user 'alice'\n+marshal_dump() {\n+\twhat=$1\n+\tpython -c 'import marshal, sys; d = marshal.load(sys.stdin); print d[\"'$what'\"]'\n+}\n+\n+# Test username support, submitting as user 'alice'.  Second user will be\n+# whoever is running these tests.  Cannot avoid having p4d auto-create that\n+# user, and the eval version only supports two users.\n test_expect_success 'preserve users' '\n+\tbobuser=$(p4 -G users | marshal_dump User) &&\n+\tbobemail=$(p4 -G users | marshal_dump Email) &&\n \tp4_add_user alice Alice &&\n-\tp4_add_user bob Bob &&\n \tp4_grant_admin alice &&\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \techo \"username: a change by alice\" >> file1 &&\n-\techo \"username: a change by bob\" >> file2 &&\n+\techo \"username: a change by $bobuser\" >> file2 &&\n \tgit commit --author \"Alice <alice@localhost>\" -m \"a change by alice\" file1 &&\n-\tgit commit --author \"Bob <bob@localhost>\" -m \"a change by bob\" file2 &&\n+\tgit commit --author \"$bobuser <$bobemail>\" -m \"a change by $bobuser\" file2 &&\n \tgit config git-p4.skipSubmitEditCheck true &&\n \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n \tp4_check_commit_author file1 alice &&\n-\tp4_check_commit_author file2 bob\n+\tp4_check_commit_author file2 $bobuser\n '\n \n # Test username support, submitting as bob, who lacks admin rights. Should\n@@ -191,9 +206,10 @@ test_expect_success 'refuse to preserve users without perms' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n-\techo \"username-noperms: a change by alice\" >> file1 &&\n-\tgit commit --author \"Alice <alice@localhost>\" -m \"perms: a change by alice\" file1 &&\n-\t! P4EDITOR=touch P4USER=bob P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n+\tp4_ungrant_admin alice &&\n+\techo \"username-noperms: a change by $bobuser\" >> file1 &&\n+\tgit commit --author \"$bobuser <$bobemail>\" -m \"perms: a change by $bobuser\" file1 &&\n+\t! P4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n \t! git diff --exit-code HEAD..p4/master > /dev/null\n '\n \n@@ -202,9 +218,10 @@ test_expect_success 'preserve user where author is unknown to p4' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n-\tgit config git-p4.skipSubmitEditCheck true\n-\techo \"username-bob: a change by bob\" >> file1 &&\n-\tgit commit --author \"Bob <bob@localhost>\" -m \"preserve: a change by bob\" file1 &&\n+\tgit config git-p4.skipSubmitEditCheck true &&\n+\tp4_grant_admin alice &&\n+\techo \"username-bob: a change by $bobuser\" >> file1 &&\n+\tgit commit --author \"$bobuser <$bobemail>\" -m \"preserve: a change by $bobuser\" file1 &&\n \techo \"username-unknown: a change by charlie\" >> file1 &&\n \tgit commit --author \"Charlie <charlie@localhost>\" -m \"preserve: a change by charlie\" file1 &&\n \t! P4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n@@ -226,11 +243,10 @@ test_expect_success 'not preserving user with mixed authorship' '\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit config git-p4.skipSubmitEditCheck true &&\n-\tp4_add_user derek Derek &&\n \n-\tmake_change_by_user usernamefile3 Derek derek@localhost &&\n+\tmake_change_by_user usernamefile3 $bobuser $bobemail &&\n \tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\tgrep \"git author derek@localhost does not match\" actual &&\n+\tgrep \"git author $bobemail does not match\" actual &&\n \n \tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n \tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-- \n1.7.5.4\n"},{"id":"172418","messageId":"20110731003936.GD4867@arf.padd.com","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"[PATCH 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T00:39:36Z","receivedAt":"2011-07-31T00:39:36Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Add this missing line in one of the tests.  Otherwise, on fast\nmachines, the following git-p4 commit will complain that nobody\nedited the submission message.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9800-git-p4.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 04d8413..24a8b79 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -207,6 +207,7 @@ test_expect_success 'refuse to preserve users without perms' '\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tp4_ungrant_admin alice &&\n+\tgit config git-p4.skipSubmitEditCheck true &&\n \techo \"username-noperms: a change by $bobuser\" >> file1 &&\n \tgit commit --author \"$bobuser <$bobemail>\" -m \"perms: a change by $bobuser\" file1 &&\n \t! P4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n-- \n1.7.5.4\n"},{"id":"172419","messageId":"20110731003953.GE4867@arf.padd.com","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"[PATCH 4/4] git-p4: commit time should be most recent p4 change time","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T00:39:53Z","receivedAt":"2011-07-31T00:39:53Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When importing a repo, the time on the initial commit had been\njust \"now\".  But this causes problems when trying to share among\ngit-p4 repos that were created identically, although at different\ntimes.  Instead, use the time in the top-most p4 change as the\ntime for the git import commit.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n contrib/fast-import/git-p4 |   15 ++++++++++++++-\n t/t9800-git-p4.sh          |   14 ++++++++++++++\n 2 files changed, 28 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 98d2aee..6b9de9e 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):\n     def importHeadRevision(self, revision):\n         print \"Doing initial import of %s from revision %s into %s\" % (' '.join(self.depotPaths), revision, self.branch)\n \n-        details = { \"user\" : \"git perforce import user\", \"time\" : int(time.time()) }\n+        details = {}\n+        details[\"user\"] = \"git perforce import user\"\n         details[\"desc\"] = (\"Initial import of %s from the state at revision %s\\n\"\n                            % (' '.join(self.depotPaths), revision))\n         details[\"change\"] = revision\n@@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):\n             fileCnt = fileCnt + 1\n \n         details[\"change\"] = newestRevision\n+\n+        # Use time from top-most change so that all git-p4 clones of\n+        # the same p4 repo have the same commit SHA1s.\n+        res = p4CmdList(\"describe -s %d\" % newestRevision)\n+        newestTime = None\n+        for r in res:\n+            if r.has_key('time'):\n+                newestTime = int(r['time'])\n+        if newestTime is None:\n+            die(\"\\\"describe -s\\\" on newest change %d did not give a time\")\n+        details[\"time\"] = newestTime\n+\n         self.updateOptionDict(details)\n         try:\n             self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 24a8b79..1b5f4b2 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -265,6 +265,20 @@ test_expect_success 'not preserving user with mixed authorship' '\n \tp4_check_commit_author usernamefile3 alice\n '\n \n+# Sleep a bit so that the top-most p4 change did not happen \"now\".  Then\n+# import the repo and make sure that the initial import has the same time\n+# as the top-most change.\n+test_expect_success 'initial import time from top change time' '\n+\tp4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change) &&\n+\tp4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time) &&\n+\tsleep 3 &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\tcd \"$git\" &&\n+\tgittime=$(git show -s --raw --pretty=format:%at HEAD) &&\n+\techo $p4time $gittime &&\n+\ttest $p4time = $gittime\n+'\n \n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n-- \n1.7.5.4\n"},{"id":"172425","messageId":"4E352535.4060804@diamand.org","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"Re: [PATCH 0/4] git-p4 test cleanup, commit time change","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T09:49:41Z","receivedAt":"2011-07-31T09:49:41Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31/07/11 01:35, Pete Wyckoff wrote:\n> Here's a small patch series to clean up the git-p4 tests a bit,\n> make them work on a 2-user eval p4d, and to fix a problem with\n> the timestamp on the import commit.\n>\n> Luke, if you can take a look and ack these, I'd appreciate\n> the review since I had to adjust your --preserveUser tests.\n> And if you're looking at that one, might as well look at them\n> all.  :)\n>\n> I'd welcome review from anyone with interest, and will plan\n> to submit these to Junio in a week or so.\n\nJust looking at them now.\n\nOne thing though - are you sure the p4d demo license only allows 2 \nusers? I've only ever used the demo p4d for testing and it seems to work \nfine (just tried again with version P4D/LINUX26X86_64/2010.2/322263).\n\nThe web page says this:\n\n\"The free Perforce Server supports two users and five client workspaces, \nor unlimited users and up to 1,000 files. Request a 45-day evaluation \nlicense to support any number of users and unlimited files.\"\n\nWhich I assume means the test harness couldn't have more than 1000 files \nand some unspecified number of client workspaces?\n\nI've just created 4 users by hand with no obvious problems. I'm pretty \nsure I'm not accidentally using the corporate server!\n\nConfuzzled!\n"},{"id":"172426","messageId":"4E352780.5040206@diamand.org","threadId":"27966","inReplyTo":"20110731003855.GB4867@arf.padd.com","subject":"Re: [PATCH 1/4] git-p4: use test_when_finished in tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T09:59:28Z","receivedAt":"2011-07-31T09:59:28Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31/07/11 01:38, Pete Wyckoff wrote:\n> Cleanup nicely when tests fail.  This avoids many duplicated\n> lines in the tests, and adds cleanup in a couple of tests that\n> did not have it.  When one fails, now all the rest will not\n> fail too.\n\nAcked.\n\nMuch tidier.\n\n>\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------\n>   1 files changed, 49 insertions(+), 54 deletions(-)\n>\n> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\n> index 33b0127..aec3ba1 100755\n> --- a/t/t9800-git-p4.sh\n> +++ b/t/t9800-git-p4.sh\n> @@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '\n>   \tcd \"$TRASH_DIRECTORY\"\n>   '\n>\n> +cleanup_git() {\n> +\tcd \"$TRASH_DIRECTORY\"&&\n> +\trm -rf \"$git\"&&\n> +\tmkdir \"$git\"\n> +}\n> +\n>   test_expect_success 'basic git-p4 clone' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \tgit log --oneline>lines&&\n> -\ttest_line_count = 1 lines&&\n> -\tcd ..&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest_line_count = 1 lines\n>   '\n>\n>   test_expect_success 'git-p4 clone @all' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot@all&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \tgit log --oneline>lines&&\n> -\ttest_line_count = 2 lines&&\n> -\tcd ..&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest_line_count = 2 lines\n>   '\n>\n>   test_expect_success 'git-p4 sync uninitialized repo' '\n>   \ttest_create_repo \"$git\"&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n> -\ttest_must_fail \"$GITP4\" sync&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest_must_fail \"$GITP4\" sync\n>   '\n>\n>   #\n> @@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '\n>   #\n>   test_expect_success 'git-p4 sync new branch' '\n>   \ttest_create_repo \"$git\"&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \ttest_commit head&&\n>   \t\"$GITP4\" sync --branch=refs/remotes/p4/depot //depot@all&&\n>   \tgit log --oneline p4/depot>lines&&\n> -\tcat lines&&\n> -\ttest_line_count = 2 lines&&\n> -\tcd ..&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest_line_count = 2 lines\n>   '\n>\n>   test_expect_success 'exit when p4 fails to produce marshaled output' '\n>   \tbadp4dir=\"$TRASH_DIRECTORY/badp4dir\"&&\n>   \tmkdir -p \"$badp4dir\"&&\n> +\ttest_when_finished \"rm -rf $badp4dir\"&&\n>   \tcat>\"$badp4dir\"/p4<<-EOF&&\n>   \t#!$SHELL_PATH\n>   \texit 1\n> @@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '\n>   \techo file-wild-at>file-wild@at&&\n>   \techo file-wild-percent>file-wild%percent&&\n>   \tp4 add -f file-wild*&&\n> -\tp4 submit -d \"file wildcards\"&&\n> -\tcd \"$TRASH_DIRECTORY\"\n> +\tp4 submit -d \"file wildcards\"\n>   '\n>\n>   test_expect_success 'wildcard files git-p4 clone' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \ttest -f file-wild#hash&&\n>   \ttest -f file-wild\\*star&&\n>   \ttest -f file-wild@at&&\n> -\ttest -f file-wild%percent&&\n> -\tcd \"$TRASH_DIRECTORY\"&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest -f file-wild%percent\n>   '\n>\n>   test_expect_success 'clone bare' '\n>   \t\"$GITP4\" clone --dest=\"$git\" --bare //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \ttest ! -d .git&&\n>   \tbare=`git config --get core.bare`&&\n> -\ttest \"$bare\" = true&&\n> -\tcd \"$TRASH_DIRECTORY\"&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest \"$bare\" = true\n>   '\n>\n>   p4_add_user() {\n> @@ -173,6 +173,7 @@ test_expect_success 'preserve users' '\n>   \tp4_add_user bob Bob&&\n>   \tp4_grant_admin alice&&\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \techo \"username: a change by alice\">>  file1&&\n>   \techo \"username: a change by bob\">>  file2&&\n> @@ -181,27 +182,25 @@ test_expect_success 'preserve users' '\n>   \tgit config git-p4.skipSubmitEditCheck true&&\n>   \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user&&\n>   \tp4_check_commit_author file1 alice&&\n> -\tp4_check_commit_author file2 bob&&\n> -\tcd \"$TRASH_DIRECTORY\"&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\tp4_check_commit_author file2 bob\n>   '\n>\n>   # Test username support, submitting as bob, who lacks admin rights. Should\n>   # not submit change to p4 (git diff should show deltas).\n>   test_expect_success 'refuse to preserve users without perms' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \techo \"username-noperms: a change by alice\">>  file1&&\n>   \tgit commit --author \"Alice<alice@localhost>\" -m \"perms: a change by alice\" file1&&\n>   \t! P4EDITOR=touch P4USER=bob P4PASSWD=secret \"$GITP4\" commit --preserve-user&&\n> -\t! git diff --exit-code HEAD..p4/master>  /dev/null&&\n> -\tcd \"$TRASH_DIRECTORY\"&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\t! git diff --exit-code HEAD..p4/master>  /dev/null\n>   '\n>\n>   # What happens with unknown author? Without allowMissingP4Users it should fail.\n>   test_expect_success 'preserve user where author is unknown to p4' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \tgit config git-p4.skipSubmitEditCheck true\n>   \techo \"username-bob: a change by bob\">>  file1&&\n> @@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '\n>   \tgit config git-p4.preserveUser true&&\n>   \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit&&\n>   \tgit diff --exit-code HEAD..p4/master>  /dev/null&&\n> -\tp4_check_commit_author file1 alice&&\n> -\tcd \"$TRASH_DIRECTORY\"&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\tp4_check_commit_author file1 alice\n>   '\n>\n>   # If we're *not* using --preserve-user, git-p4 should warn if we're submitting\n> @@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '\n>   # Test: warning disabled and user is the same.\n>   test_expect_success 'not preserving user with mixed authorship' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> -\t(\n> -\t\tcd \"$git\"&&\n> -\t\tgit config git-p4.skipSubmitEditCheck true&&\n> -\t\tp4_add_user derek Derek&&\n> -\n> -\t\tmake_change_by_user usernamefile3 Derek derek@localhost&&\n> -\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> -\t\tgrep \"git author derek@localhost does not match\" actual&&\n> -\n> -\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost&&\n> -\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> -\t\tgrep \"git author charlie@localhost does not match\" actual&&\n> -\n> -\t\tmake_change_by_user usernamefile3 alice alice@localhost&&\n> -\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> -\t\t! grep \"git author.*does not match\" actual&&\n> -\n> -\t\tgit config git-p4.skipUserNameCheck true&&\n> -\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost&&\n> -\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> -\t\t! grep \"git author.*does not match\" actual&&\n> -\n> -\t\tp4_check_commit_author usernamefile3 alice\n> -\t)&&\n> -\trm -rf \"$git\"&&  mkdir \"$git\"\n> +\ttest_when_finished cleanup_git&&\n> +\tcd \"$git\"&&\n> +\tgit config git-p4.skipSubmitEditCheck true&&\n> +\tp4_add_user derek Derek&&\n> +\n> +\tmake_change_by_user usernamefile3 Derek derek@localhost&&\n> +\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> +\tgrep \"git author derek@localhost does not match\" actual&&\n> +\n> +\tmake_change_by_user usernamefile3 Charlie charlie@localhost&&\n> +\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> +\tgrep \"git author charlie@localhost does not match\" actual&&\n> +\n> +\tmake_change_by_user usernamefile3 alice alice@localhost&&\n> +\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> +\t! grep \"git author.*does not match\" actual&&\n> +\n> +\tgit config git-p4.skipUserNameCheck true&&\n> +\tmake_change_by_user usernamefile3 Charlie charlie@localhost&&\n> +\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit>actual&&\n> +\t! grep \"git author.*does not match\" actual&&\n> +\n> +\tp4_check_commit_author usernamefile3 alice\n>   '\n>\n>\n"},{"id":"172427","messageId":"4E3527E6.8080303@diamand.org","threadId":"27966","inReplyTo":"20110731003936.GD4867@arf.padd.com","subject":"Re: [PATCH 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T10:01:10Z","receivedAt":"2011-07-31T10:01:10Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31/07/11 01:39, Pete Wyckoff wrote:\n> Add this missing line in one of the tests.  Otherwise, on fast\n> machines, the following git-p4 commit will complain that nobody\n> edited the submission message.\n\nThanks for fixing this.\n\nAcked.\n\n\n>\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   t/t9800-git-p4.sh |    1 +\n>   1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\n> index 04d8413..24a8b79 100755\n> --- a/t/t9800-git-p4.sh\n> +++ b/t/t9800-git-p4.sh\n> @@ -207,6 +207,7 @@ test_expect_success 'refuse to preserve users without perms' '\n>   \ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n>   \tp4_ungrant_admin alice&&\n> +\tgit config git-p4.skipSubmitEditCheck true&&\n>   \techo \"username-noperms: a change by $bobuser\">>  file1&&\n>   \tgit commit --author \"$bobuser<$bobemail>\" -m \"perms: a change by $bobuser\" file1&&\n>   \t! P4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user&&\n"},{"id":"172428","messageId":"4E352B1F.4070402@diamand.org","threadId":"27966","inReplyTo":"20110731003953.GE4867@arf.padd.com","subject":"Re: [PATCH 4/4] git-p4: commit time should be most recent p4 change time","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T10:14:55Z","receivedAt":"2011-07-31T10:14:55Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31/07/11 01:39, Pete Wyckoff wrote:\n> When importing a repo, the time on the initial commit had been\n> just \"now\".  But this causes problems when trying to share among\n> git-p4 repos that were created identically, although at different\n> times.  Instead, use the time in the top-most p4 change as the\n> time for the git import commit.\n\nAck.\n\n\n>\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   contrib/fast-import/git-p4 |   15 ++++++++++++++-\n>   t/t9800-git-p4.sh          |   14 ++++++++++++++\n>   2 files changed, 28 insertions(+), 1 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 98d2aee..6b9de9e 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):\n>       def importHeadRevision(self, revision):\n>           print \"Doing initial import of %s from revision %s into %s\" % (' '.join(self.depotPaths), revision, self.branch)\n>\n> -        details = { \"user\" : \"git perforce import user\", \"time\" : int(time.time()) }\n> +        details = {}\n> +        details[\"user\"] = \"git perforce import user\"\n>           details[\"desc\"] = (\"Initial import of %s from the state at revision %s\\n\"\n>                              % (' '.join(self.depotPaths), revision))\n>           details[\"change\"] = revision\n> @@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):\n>               fileCnt = fileCnt + 1\n>\n>           details[\"change\"] = newestRevision\n> +\n> +        # Use time from top-most change so that all git-p4 clones of\n> +        # the same p4 repo have the same commit SHA1s.\n> +        res = p4CmdList(\"describe -s %d\" % newestRevision)\n> +        newestTime = None\n> +        for r in res:\n> +            if r.has_key('time'):\n> +                newestTime = int(r['time'])\n> +        if newestTime is None:\n> +            die(\"\\\"describe -s\\\" on newest change %d did not give a time\")\n> +        details[\"time\"] = newestTime\n> +\n>           self.updateOptionDict(details)\n>           try:\n>               self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)\n> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\n> index 24a8b79..1b5f4b2 100755\n> --- a/t/t9800-git-p4.sh\n> +++ b/t/t9800-git-p4.sh\n> @@ -265,6 +265,20 @@ test_expect_success 'not preserving user with mixed authorship' '\n>   \tp4_check_commit_author usernamefile3 alice\n>   '\n>\n> +# Sleep a bit so that the top-most p4 change did not happen \"now\".  Then\n> +# import the repo and make sure that the initial import has the same time\n> +# as the top-most change.\n> +test_expect_success 'initial import time from top change time' '\n> +\tp4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change)&&\n> +\tp4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time)&&\n> +\tsleep 3&&\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n> +\tcd \"$git\"&&\n> +\tgittime=$(git show -s --raw --pretty=format:%at HEAD)&&\n> +\techo $p4time $gittime&&\n> +\ttest $p4time = $gittime\n> +'\n>\n>   test_expect_success 'shutdown' '\n>   \tpid=`pgrep -f p4d`&&\n"},{"id":"172443","messageId":"20110731134237.GA6564@arf.padd.com","threadId":"27966","inReplyTo":"4E352535.4060804@diamand.org","subject":"Re: [PATCH 0/4] git-p4 test cleanup, commit time change","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:42:37Z","receivedAt":"2011-07-31T13:42:37Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Sun, 31 Jul 2011 10:49 +0100:\n> One thing though - are you sure the p4d demo license only allows 2\n> users? I've only ever used the demo p4d for testing and it seems to\n> work fine (just tried again with version\n> P4D/LINUX26X86_64/2010.2/322263).\n\nYou are so right.  I was using an older p4d (2009.1/222893) at\nhome, and didn't even think about checking for updates.\n\nThere was one line in patch 3 that should survive, just to add a\n\"&&\".  I've added your acks for the rest and will repost to show\nthe current changes.\n\nThanks,\n\n\t\t-- Pete\n"},{"id":"172444","messageId":"20110731134416.GB6564@arf.padd.com","threadId":"27966","inReplyTo":"20110731003557.GA4867@arf.padd.com","subject":"[PATCHv2 0/4] git-p4 test cleanup, commit time change","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:44:16Z","receivedAt":"2011-07-31T13:44:16Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"Re-spun, removing all of the original patch 2 now that Luke\npointed out that a new p4d is available.\n\nOriginal intro:\n\n    Here's a small patch series to clean up the git-p4 tests a bit,\n    make them work on a 2-user eval p4d, and to fix a problem with\n    the timestamp on the import commit.\n"},{"id":"172445","messageId":"20110731134450.GC6564@arf.padd.com","threadId":"27966","inReplyTo":"20110731134416.GB6564@arf.padd.com","subject":"[PATCHv2 1/4] git-p4: use test_when_finished in tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:44:50Z","receivedAt":"2011-07-31T13:44:50Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"Cleanup nicely when tests fail.  This avoids many duplicated\nlines in the tests, and adds cleanup in a couple of tests that\ndid not have it.  When one fails, now all the rest will not\nfail too.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\nAcked-by: Luke Diamand <luke@diamand.org>\n---\n t/t9800-git-p4.sh |  103 +++++++++++++++++++++++++----------------------------\n 1 files changed, 49 insertions(+), 54 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 33b0127..aec3ba1 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -45,29 +45,33 @@ test_expect_success 'add p4 files' '\n \tcd \"$TRASH_DIRECTORY\"\n '\n \n+cleanup_git() {\n+\tcd \"$TRASH_DIRECTORY\" &&\n+\trm -rf \"$git\" &&\n+\tmkdir \"$git\"\n+}\n+\n test_expect_success 'basic git-p4 clone' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit log --oneline >lines &&\n-\ttest_line_count = 1 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 1 lines\n '\n \n test_expect_success 'git-p4 clone @all' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit log --oneline >lines &&\n-\ttest_line_count = 2 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 2 lines\n '\n \n test_expect_success 'git-p4 sync uninitialized repo' '\n \ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n-\ttest_must_fail \"$GITP4\" sync &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_must_fail \"$GITP4\" sync\n '\n \n #\n@@ -76,19 +80,18 @@ test_expect_success 'git-p4 sync uninitialized repo' '\n #\n test_expect_success 'git-p4 sync new branch' '\n \ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest_commit head &&\n \t\"$GITP4\" sync --branch=refs/remotes/p4/depot //depot@all &&\n \tgit log --oneline p4/depot >lines &&\n-\tcat lines &&\n-\ttest_line_count = 2 lines &&\n-\tcd .. &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_line_count = 2 lines\n '\n \n test_expect_success 'exit when p4 fails to produce marshaled output' '\n \tbadp4dir=\"$TRASH_DIRECTORY/badp4dir\" &&\n \tmkdir -p \"$badp4dir\" &&\n+\ttest_when_finished \"rm -rf $badp4dir\" &&\n \tcat >\"$badp4dir\"/p4 <<-EOF &&\n \t#!$SHELL_PATH\n \texit 1\n@@ -106,29 +109,26 @@ test_expect_success 'add p4 files with wildcards in the names' '\n \techo file-wild-at >file-wild@at &&\n \techo file-wild-percent >file-wild%percent &&\n \tp4 add -f file-wild* &&\n-\tp4 submit -d \"file wildcards\" &&\n-\tcd \"$TRASH_DIRECTORY\"\n+\tp4 submit -d \"file wildcards\"\n '\n \n test_expect_success 'wildcard files git-p4 clone' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest -f file-wild#hash &&\n \ttest -f file-wild\\*star &&\n \ttest -f file-wild@at &&\n-\ttest -f file-wild%percent &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest -f file-wild%percent\n '\n \n test_expect_success 'clone bare' '\n \t\"$GITP4\" clone --dest=\"$git\" --bare //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \ttest ! -d .git &&\n \tbare=`git config --get core.bare` &&\n-\ttest \"$bare\" = true &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest \"$bare\" = true\n '\n \n p4_add_user() {\n@@ -173,6 +173,7 @@ test_expect_success 'preserve users' '\n \tp4_add_user bob Bob &&\n \tp4_grant_admin alice &&\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \techo \"username: a change by alice\" >> file1 &&\n \techo \"username: a change by bob\" >> file2 &&\n@@ -181,27 +182,25 @@ test_expect_success 'preserve users' '\n \tgit config git-p4.skipSubmitEditCheck true &&\n \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n \tp4_check_commit_author file1 alice &&\n-\tp4_check_commit_author file2 bob &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\tp4_check_commit_author file2 bob\n '\n \n # Test username support, submitting as bob, who lacks admin rights. Should\n # not submit change to p4 (git diff should show deltas).\n test_expect_success 'refuse to preserve users without perms' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \techo \"username-noperms: a change by alice\" >> file1 &&\n \tgit commit --author \"Alice <alice@localhost>\" -m \"perms: a change by alice\" file1 &&\n \t! P4EDITOR=touch P4USER=bob P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n-\t! git diff --exit-code HEAD..p4/master > /dev/null &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\t! git diff --exit-code HEAD..p4/master > /dev/null\n '\n \n # What happens with unknown author? Without allowMissingP4Users it should fail.\n test_expect_success 'preserve user where author is unknown to p4' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n \tgit config git-p4.skipSubmitEditCheck true\n \techo \"username-bob: a change by bob\" >> file1 &&\n@@ -215,9 +214,7 @@ test_expect_success 'preserve user where author is unknown to p4' '\n \tgit config git-p4.preserveUser true &&\n \tP4EDITOR=touch P4USER=alice P4PASSWD=secret \"$GITP4\" commit &&\n \tgit diff --exit-code HEAD..p4/master > /dev/null &&\n-\tp4_check_commit_author file1 alice &&\n-\tcd \"$TRASH_DIRECTORY\" &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\tp4_check_commit_author file1 alice\n '\n \n # If we're *not* using --preserve-user, git-p4 should warn if we're submitting\n@@ -226,31 +223,29 @@ test_expect_success 'preserve user where author is unknown to p4' '\n # Test: warning disabled and user is the same.\n test_expect_success 'not preserving user with mixed authorship' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n-\t(\n-\t\tcd \"$git\" &&\n-\t\tgit config git-p4.skipSubmitEditCheck true &&\n-\t\tp4_add_user derek Derek &&\n-\n-\t\tmake_change_by_user usernamefile3 Derek derek@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\tgrep \"git author derek@localhost does not match\" actual &&\n-\n-\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\tgrep \"git author charlie@localhost does not match\" actual &&\n-\n-\t\tmake_change_by_user usernamefile3 alice alice@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\t! grep \"git author.*does not match\" actual &&\n-\n-\t\tgit config git-p4.skipUserNameCheck true &&\n-\t\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n-\t\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n-\t\t! grep \"git author.*does not match\" actual &&\n-\n-\t\tp4_check_commit_author usernamefile3 alice\n-\t) &&\n-\trm -rf \"$git\" && mkdir \"$git\"\n+\ttest_when_finished cleanup_git &&\n+\tcd \"$git\" &&\n+\tgit config git-p4.skipSubmitEditCheck true &&\n+\tp4_add_user derek Derek &&\n+\n+\tmake_change_by_user usernamefile3 Derek derek@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\tgrep \"git author derek@localhost does not match\" actual &&\n+\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\tgrep \"git author charlie@localhost does not match\" actual &&\n+\n+\tmake_change_by_user usernamefile3 alice alice@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\t! grep \"git author.*does not match\" actual &&\n+\n+\tgit config git-p4.skipUserNameCheck true &&\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\tP4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit >actual &&\n+\t! grep \"git author.*does not match\" actual &&\n+\n+\tp4_check_commit_author usernamefile3 alice\n '\n \n \n-- \n1.7.5.4\n"},{"id":"172446","messageId":"20110731134517.GD6564@arf.padd.com","threadId":"27966","inReplyTo":"20110731134416.GB6564@arf.padd.com","subject":"[PATCHv2 2/4] git-p4: add missing && in test","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:45:17Z","receivedAt":"2011-07-31T13:45:17Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"Signed-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/t9800-git-p4.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex aec3ba1..b7eda82 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -202,7 +202,7 @@ test_expect_success 'preserve user where author is unknown to p4' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n-\tgit config git-p4.skipSubmitEditCheck true\n+\tgit config git-p4.skipSubmitEditCheck true &&\n \techo \"username-bob: a change by bob\" >> file1 &&\n \tgit commit --author \"Bob <bob@localhost>\" -m \"preserve: a change by bob\" file1 &&\n \techo \"username-unknown: a change by charlie\" >> file1 &&\n-- \n1.7.5.4\n"},{"id":"172447","messageId":"20110731134538.GE6564@arf.padd.com","threadId":"27966","inReplyTo":"20110731134416.GB6564@arf.padd.com","subject":"[PATCHv2 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:45:38Z","receivedAt":"2011-07-31T13:45:38Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"Add this missing line in one of the tests.  Otherwise, on fast\nmachines, the following git-p4 commit will complain that nobody\nedited the submission message.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\nAcked-by: Luke Diamand <luke@diamand.org>\n---\n t/t9800-git-p4.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex b7eda82..b304707 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -191,6 +191,7 @@ test_expect_success 'refuse to preserve users without perms' '\n \t\"$GITP4\" clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \tcd \"$git\" &&\n+\tgit config git-p4.skipSubmitEditCheck true &&\n \techo \"username-noperms: a change by alice\" >> file1 &&\n \tgit commit --author \"Alice <alice@localhost>\" -m \"perms: a change by alice\" file1 &&\n \t! P4EDITOR=touch P4USER=bob P4PASSWD=secret \"$GITP4\" commit --preserve-user &&\n-- \n1.7.5.4\n"},{"id":"172448","messageId":"20110731134555.GF6564@arf.padd.com","threadId":"27966","inReplyTo":"20110731134416.GB6564@arf.padd.com","subject":"[PATCHv2 4/4] git-p4: commit time should be most recent p4 change time","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-07-31T13:45:55Z","receivedAt":"2011-07-31T13:45:55Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"When importing a repo, the time on the initial commit had been\njust \"now\".  But this causes problems when trying to share among\ngit-p4 repos that were created identically, although at different\ntimes.  Instead, use the time in the top-most p4 change as the\ntime for the git import commit.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\nAcked-by: Luke Diamand <luke@diamand.org>\n---\n contrib/fast-import/git-p4 |   15 ++++++++++++++-\n t/t9800-git-p4.sh          |   19 +++++++++++++++++++\n 2 files changed, 33 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 98d2aee..6b9de9e 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1649,7 +1649,8 @@ class P4Sync(Command, P4UserMap):\n     def importHeadRevision(self, revision):\n         print \"Doing initial import of %s from revision %s into %s\" % (' '.join(self.depotPaths), revision, self.branch)\n \n-        details = { \"user\" : \"git perforce import user\", \"time\" : int(time.time()) }\n+        details = {}\n+        details[\"user\"] = \"git perforce import user\"\n         details[\"desc\"] = (\"Initial import of %s from the state at revision %s\\n\"\n                            % (' '.join(self.depotPaths), revision))\n         details[\"change\"] = revision\n@@ -1689,6 +1690,18 @@ class P4Sync(Command, P4UserMap):\n             fileCnt = fileCnt + 1\n \n         details[\"change\"] = newestRevision\n+\n+        # Use time from top-most change so that all git-p4 clones of\n+        # the same p4 repo have the same commit SHA1s.\n+        res = p4CmdList(\"describe -s %d\" % newestRevision)\n+        newestTime = None\n+        for r in res:\n+            if r.has_key('time'):\n+                newestTime = int(r['time'])\n+        if newestTime is None:\n+            die(\"\\\"describe -s\\\" on newest change %d did not give a time\")\n+        details[\"time\"] = newestTime\n+\n         self.updateOptionDict(details)\n         try:\n             self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)\ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex b304707..97ec975 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -249,6 +249,25 @@ test_expect_success 'not preserving user with mixed authorship' '\n \tp4_check_commit_author usernamefile3 alice\n '\n \n+marshal_dump() {\n+\twhat=$1\n+\tpython -c 'import marshal, sys; d = marshal.load(sys.stdin); print d[\"'$what'\"]'\n+}\n+\n+# Sleep a bit so that the top-most p4 change did not happen \"now\".  Then\n+# import the repo and make sure that the initial import has the same time\n+# as the top-most change.\n+test_expect_success 'initial import time from top change time' '\n+\tp4change=$(p4 -G changes -m 1 //depot/... | marshal_dump change) &&\n+\tp4time=$(p4 -G changes -m 1 //depot/... | marshal_dump time) &&\n+\tsleep 3 &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\tcd \"$git\" &&\n+\tgittime=$(git show -s --raw --pretty=format:%at HEAD) &&\n+\techo $p4time $gittime &&\n+\ttest $p4time = $gittime\n+'\n \n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n-- \n1.7.5.4\n"},{"id":"172451","messageId":"4E355EBF.2070400@diamand.org","threadId":"27966","inReplyTo":"20110731134517.GD6564@arf.padd.com","subject":"Re: [PATCHv2 2/4] git-p4: add missing && in test","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T13:55:11Z","receivedAt":"2011-07-31T13:55:11Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Acked.\n\n\n\nOn 31/07/11 14:45, Pete Wyckoff wrote:\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   t/t9800-git-p4.sh |    2 +-\n>   1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\n> index aec3ba1..b7eda82 100755\n> --- a/t/t9800-git-p4.sh\n> +++ b/t/t9800-git-p4.sh\n> @@ -202,7 +202,7 @@ test_expect_success 'preserve user where author is unknown to p4' '\n>   \t\"$GITP4\" clone --dest=\"$git\" //depot&&\n>   \ttest_when_finished cleanup_git&&\n>   \tcd \"$git\"&&\n> -\tgit config git-p4.skipSubmitEditCheck true\n> +\tgit config git-p4.skipSubmitEditCheck true&&\n>   \techo \"username-bob: a change by bob\">>  file1&&\n>   \tgit commit --author \"Bob<bob@localhost>\" -m \"preserve: a change by bob\" file1&&\n>   \techo \"username-unknown: a change by charlie\">>  file1&&\n"},{"id":"172456","messageId":"4E357C48.5090409@diamand.org","threadId":"27966","inReplyTo":"20110731134237.GA6564@arf.padd.com","subject":"Re: [PATCH 0/4] git-p4 test cleanup, commit time change","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-07-31T16:01:12Z","receivedAt":"2011-07-31T16:01:12Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 31/07/11 14:42, Pete Wyckoff wrote:\n> luke@diamand.org wrote on Sun, 31 Jul 2011 10:49 +0100:\n>> One thing though - are you sure the p4d demo license only allows 2\n>> users? I've only ever used the demo p4d for testing and it seems to\n>> work fine (just tried again with version\n>> P4D/LINUX26X86_64/2010.2/322263).\n>\n> You are so right.  I was using an older p4d (2009.1/222893) at\n> home, and didn't even think about checking for updates.\n>\n> There was one line in patch 3 that should survive, just to add a\n> \"&&\".  I've added your acks for the rest and will repost to show\n> the current changes.\n\nAll looks good to me!\n\nLuke\n"}]}