{"thread":{"id":"28038","subject":"[PATCH 0/4] git-p4 test cleanup, commit time change","startedAt":"2011-08-07T13:31:44Z","lastAt":"2011-08-07T13:37:19Z","messageCount":5,"participants":["Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"173090","messageId":"20110807133144.GA27521@arf.padd.com","threadId":"28038","inReplyTo":null,"subject":"[PATCH 0/4] git-p4 test cleanup, commit time change","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-08-07T13:31:44Z","receivedAt":"2011-08-07T13:31:44Z","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\n [PATCH 1/4] git-p4: use test_when_finished in tests\n [PATCH 2/4] git-p4: add missing && in test\n [PATCH 3/4] git-p4: one test missing config\n [PATCH 4/4] git-p4: commit time should be most recent p4 change time\n\n contrib/fast-import/git-p4 |   15 +++++\n t/t9800-git-p4.sh          |  125 +++++++++++++++++++++++++--------------------\n 2 files changed, 84 insertions(+), 56 deletions(-)\n\nLuke acked them, and I've been using it for a couple of weeks.  Could you\ninclude them in the next convenient release?  Thanks,\n\n\t\t-- Pete\n"},{"id":"173091","messageId":"20110807133241.GA27737@arf.padd.com","threadId":"28038","inReplyTo":"20110807133144.GA27521@arf.padd.com","subject":"[PATCH 1/4] git-p4: use test_when_finished in tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-08-07T13:32:41Z","receivedAt":"2011-08-07T13:32:41Z","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>\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":"173092","messageId":"20110807133425.GB27737@arf.padd.com","threadId":"28038","inReplyTo":"20110807133144.GA27521@arf.padd.com","subject":"[PATCH 2/4] git-p4: add missing && in test","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-08-07T13:34:25Z","receivedAt":"2011-08-07T13:34:25Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Signed-off-by: Pete Wyckoff <pw@padd.com>\nAcked-by: Luke Diamand <luke@diamand.org>\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":"173093","messageId":"20110807133548.GC27737@arf.padd.com","threadId":"28038","inReplyTo":"20110807133144.GA27521@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-08-07T13:35:48Z","receivedAt":"2011-08-07T13:35:48Z","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>\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":"173094","messageId":"20110807133719.GD27737@arf.padd.com","threadId":"28038","inReplyTo":"20110807133144.GA27521@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-08-07T13:37:19Z","receivedAt":"2011-08-07T13:37:19Z","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>\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"}]}