{"thread":{"id":"27320","subject":"[PATCH/RFC v4] git-p4: warn if git authorship won't be retained","startedAt":"2011-05-11T00:26:00Z","lastAt":"2011-05-11T00:26:01Z","messageCount":2,"participants":["Luke Diamand"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"167650","messageId":"1305073561-24910-1-git-send-email-luke@diamand.org","threadId":"27320","inReplyTo":null,"subject":"[PATCH/RFC v4] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-11T00:26:00Z","receivedAt":"2011-05-11T00:26:00Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Updated version of git-p4 authorship warning suggested by Junio,\nincorporating fixes for problems in v2 pointed out by Peter Wyckoff.\n\nIn the previous version of this patch I forgot to add an && in the\ntest harness.\n\nPete suggested doing \"grep -q\" however it turns out that this\ndoesn't work - git-p4 explodes when it tries to do stdout.flush() at\nline 1588 in importChanges(). I can only imagine that grep -q does\nsomething very subtle to stdin.\n\nLuke Diamand (1):\n git-p4: warn if git authorship won't be retained\n\n contrib/fast-import/git-p4 | 33 +++++++++++++++++++++++++++-\n contrib/fast-import/git-p4.txt | 7 ++++++\n t/t9800-git-p4.sh | 46 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 84 insertions(+), 2 deletions(-)\n"},{"id":"167651","messageId":"1305073561-24910-2-git-send-email-luke@diamand.org","threadId":"27320","inReplyTo":"1305073561-24910-1-git-send-email-luke@diamand.org","subject":"[PATCH/RFC v4] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-11T00:26:01Z","receivedAt":"2011-05-11T00:26:01Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"If the git commits you are submitting contain changes made by\nother people, the authorship will not be retained. Change git-p4\nto warn of this and to note that --preserve-user can be used\nto solve the problem (if you have suitable permissions).\nThe warning can be disabled.\n\nAdd a test case and update documentation.\n\nRequested-by: Junio C Hamano \nAcked-by: Pete Wyckoff \nSigned-off-by: Luke Diamand \n---\n contrib/fast-import/git-p4 | 33 +++++++++++++++++++++++++++-\n contrib/fast-import/git-p4.txt | 7 ++++++\n t/t9800-git-p4.sh | 46 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 84 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex bd8c761..021b985 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -614,6 +614,7 @@ class P4Submit(Command, P4UserMap):\n self.verbose = False\n self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n self.isWindows = (platform.system() == \"Windows\")\n+ self.myP4UserId = None\n \n def check(self):\n if len(p4CmdList(\"opened ...\")) > 0:\n@@ -721,6 +722,25 @@ class P4Submit(Command, P4UserMap):\n return 1\n return 0\n \n+ def p4UserId(self):\n+ if self.myP4UserId:\n+ return self.myP4UserId\n+\n+ results = p4CmdList(\"user -o\")\n+ for r in results:\n+ if r.has_key('User'):\n+ self.myP4UserId = r['User']\n+ return r['User']\n+ die(\"Could not find your p4 user id\")\n+\n+ def p4UserIsMe(self, p4User):\n+ # return True if the given p4 user is actually me\n+ me = self.p4UserId()\n+ if not p4User or p4User != me:\n+ return False\n+ else:\n+ return True\n+\n def prepareSubmitTemplate(self):\n # remove lines in the Files section that show changes to files outside the depot path we're committing into\n template = \"\"\n@@ -750,8 +770,7 @@ class P4Submit(Command, P4UserMap):\n def applyCommit(self, id):\n print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n \n- if self.preserveUser:\n- (p4User, gitEmail) = self.p4UserForCommit(id)\n+ (p4User, gitEmail) = self.p4UserForCommit(id)\n \n if not self.detectRenames:\n # If not explicitly set check the config variable\n@@ -887,6 +906,11 @@ class P4Submit(Command, P4UserMap):\n newdiff += \"+\" + line\n f.close()\n \n+ if self.checkAuthorship and not self.p4UserIsMe(p4User):\n+ submitTemplate += \"######## git author %s does not match your p4 account.\\n\" % gitEmail\n+ submitTemplate += \"######## Use git-p4 option --preserve-user to modify authorship\\n\"\n+ submitTemplate += \"######## Use git-p4 config git-p4.skipUserNameCheck hides this message.\\n\"\n+\n separatorLine = \"######## everything below this line is just the diff #######\\n\"\n \n [handle, fileName] = tempfile.mkstemp()\n@@ -998,6 +1022,11 @@ class P4Submit(Command, P4UserMap):\n commits.append(line.strip())\n commits.reverse()\n \n+ if self.preserveUser or (gitConfig(\"git-p4.skipUserNameCheck\") == \"true\"):\n+ self.checkAuthorship = False\n+ else:\n+ self.checkAuthorship = True\n+\n if self.preserveUser:\n self.checkValidP4Users(commits)\n \ndiff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\nindex d46b7a5..3c7c138 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -226,6 +226,13 @@ stop. With allowMissingPerforceUsers set to true, git-p4 will use the\n current user (i.e. the behavior without --preserve-user) and carry on with\n the perforce commit.\n \n+git-p4.skipUserNameCheck\n+\n+ git config [--global] git-p4.skipUserNameCheck false\n+\n+When submitting, git-p4 checks that the git commits are authored by the current\n+p4 user, and warns if they are not. This disables the check.\n+\n Implementation Details...\n =========================\n \ndiff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\nindex 4fb0e24..9c10943 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -160,6 +160,15 @@ p4_check_commit_author() {\n fi\n }\n \n+make_change_by_user() {\n+\tfile=$1\n+\tname=$2\n+\temail=$3\n+\techo \"username: a change by $name\" >> $file &&\n+\tgit add $file &&\n+\tgit commit --author \"$name <$email>\" -m \"a change by $name\"\n+}\n+\n # Test username support, submitting as user 'alice'\n test_expect_success 'preserve users' '\n \tp4_add_user alice Alice &&\n@@ -213,6 +222,43 @@ test_expect_success 'preserve user where author is unknown to p4' '\n \trm -rf \"$git\" && mkdir \"$git\"\n '\n \n+# If we're *not* using --preserve-user, git-p4 should warn if we're submitting\n+# changes that are not all ours.\n+# Test: user in p4 and user 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+\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+\t(P4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit |\n+\t\tgrep \"git author derek@localhost does not match\") &&\n+\tgit diff --exit-code HEAD..p4/master &&\n+\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\t(P4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit |\n+\t\tgrep \"git author charlie@localhost does not match\") &&\n+\tgit diff --exit-code HEAD..p4/master &&\n+\n+\tmake_change_by_user usernamefile3 alice alice@localhost &&\n+\t!(P4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit |\n+\t\tgrep \"does not match\") &&\n+\tgit diff --exit-code HEAD..p4/master &&\n+\n+\tgit config git-p4.skipUserNameCheck true &&\n+\tmake_change_by_user usernamefile3 Charlie charlie@localhost &&\n+\t!(P4EDITOR=cat P4USER=alice P4PASSWD=secret \"$GITP4\" commit |\n+\t\tgrep \"git author charlie@localhost does not match\") &&\n+\tgit diff --exit-code HEAD..p4/master &&\n+\n+\tp4_check_commit_author usernamefile3 alice &&\n+\tcd \"$TRASH_DIRECTORY\" &&\n+\trm -rf \"$git\" && mkdir \"$git\"\n+'\n+\n+\n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n \ttest -n \"$pid\" &&\n-- \n1.7.1\n"}]}