# [PATCH 0/4] git-p4 test cleanup, commit time change 5 messages from 2011-08-07 to 2011-08-07. Participants: Pete Wyckoff. Thread: https://gitlist.dev/t/28038 ## Pete Wyckoff, 2011-08-07 13:31 Subject: [PATCH 0/4] git-p4 test cleanup, commit time change Message-ID: <20110807133144.GA27521@arf.padd.com> URL: https://gitlist.dev/e/20110807133144.GA27521%40arf.padd.com ``` 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. [PATCH 1/4] git-p4: use test_when_finished in tests [PATCH 2/4] git-p4: add missing && in test [PATCH 3/4] git-p4: one test missing config [PATCH 4/4] git-p4: commit time should be most recent p4 change time contrib/fast-import/git-p4 | 15 +++++ t/t9800-git-p4.sh | 125 +++++++++++++++++++++++++-------------------- 2 files changed, 84 insertions(+), 56 deletions(-) Luke acked them, and I've been using it for a couple of weeks. Could you include them in the next convenient release? Thanks, -- Pete ``` ## Pete Wyckoff, 2011-08-07 13:32 Subject: [PATCH 1/4] git-p4: use test_when_finished in tests Message-ID: <20110807133241.GA27737@arf.padd.com> URL: https://gitlist.dev/e/20110807133241.GA27737%40arf.padd.com In-Reply-To: <20110807133144.GA27521@arf.padd.com> ``` 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 Acked-by: Luke Diamand --- 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 " -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, 2011-08-07 13:34 Subject: [PATCH 2/4] git-p4: add missing && in test Message-ID: <20110807133425.GB27737@arf.padd.com> URL: https://gitlist.dev/e/20110807133425.GB27737%40arf.padd.com In-Reply-To: <20110807133144.GA27521@arf.padd.com> ``` Signed-off-by: Pete Wyckoff Acked-by: Luke Diamand --- 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 " -m "preserve: a change by bob" file1 && echo "username-unknown: a change by charlie" >> file1 && -- 1.7.5.4 ``` ## Pete Wyckoff, 2011-08-07 13:35 Subject: [PATCH 3/4] git-p4: one test missing config git-p4.skipSubmitEditCheck Message-ID: <20110807133548.GC27737@arf.padd.com> URL: https://gitlist.dev/e/20110807133548.GC27737%40arf.padd.com In-Reply-To: <20110807133144.GA27521@arf.padd.com> ``` 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 Acked-by: Luke Diamand --- 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 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 " -m "perms: a change by alice" file1 && ! P4EDITOR=touch P4USER=bob P4PASSWD=secret "$GITP4" commit --preserve-user && -- 1.7.5.4 ``` ## Pete Wyckoff, 2011-08-07 13:37 Subject: [PATCH 4/4] git-p4: commit time should be most recent p4 change time Message-ID: <20110807133719.GD27737@arf.padd.com> URL: https://gitlist.dev/e/20110807133719.GD27737%40arf.padd.com In-Reply-To: <20110807133144.GA27521@arf.padd.com> ``` 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 Acked-by: Luke Diamand --- contrib/fast-import/git-p4 | 15 ++++++++++++++- t/t9800-git-p4.sh | 19 +++++++++++++++++++ 2 files changed, 33 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 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 ```