{"thread":{"id":"27317","subject":"[PATCH/RFC v3] git-p4: warn if git authorship won't be retained","startedAt":"2011-05-10T19:27:32Z","lastAt":"2011-05-11T07:50:13Z","messageCount":6,"participants":["Luke Diamand","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"167613","messageId":"1305055653-5133-1-git-send-email-luke@diamand.org","threadId":"27317","inReplyTo":null,"subject":"[PATCH/RFC v3] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-10T19:27:32Z","receivedAt":"2011-05-10T19:27:32Z","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 Pete Wyckoff.\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":"167614","messageId":"1305055653-5133-2-git-send-email-luke@diamand.org","threadId":"27317","inReplyTo":"1305055653-5133-1-git-send-email-luke@diamand.org","subject":"[PATCH] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-10T19:27:33Z","receivedAt":"2011-05-10T19:27:33Z","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 \nHelped-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..d6c7d13 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"},{"id":"167632","messageId":"BANLkTi=hi6cZkZAAhJXM04tOk-16x78wYA@mail.gmail.com","threadId":"27317","inReplyTo":"1305055653-5133-2-git-send-email-luke@diamand.org","subject":"Re: [PATCH] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-10T22:11:39Z","receivedAt":"2011-05-10T22:11:39Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Tue, May 10, 2011 at 8:27 PM, Luke Diamand wrote:\n> If the git commits you are submitting contain changes made by\n> other people, the authorship will not be retained. Change git-p4\n> to warn of this and to note that --preserve-user can be used\n> to solve the problem (if you have suitable permissions).\n> The warning can be disabled.\n>\n\nOf course, I've forgotten the && in the test case.\n\nSigh.\n"},{"id":"167642","messageId":"7vhb92i2yu.fsf@alter.siamese.dyndns.org","threadId":"27317","inReplyTo":"1305055653-5133-2-git-send-email-luke@diamand.org","subject":"Re: [PATCH] git-p4: warn if git authorship won't be retained","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T22:51:37Z","receivedAt":"2011-05-10T22:51:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh\n> index 4fb0e24..d6c7d13 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\n\tfile=$1 name=$2 email=$3 &&\n\n> +\techo \"username: a change by $name\" >> $file &&\n> ...\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> +\tp4_check_commit_author usernamefile3 alice &&\n> +\tcd \"$TRASH_DIRECTORY\" &&\n> +\trm -rf \"$git\" && mkdir \"$git\"\n> +'\n\nWhen the commands in the && chain fails after 'cd \"$git\"' near the top but\nnot before 'cd \"$TRASH_DIRECTORY\"' near the end, you would end up starting\nthe next test from somewhere different from \"$TRASH_DIRECTORY\".\n\nDo it like this instead:\n\n\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n (\n \tcd \"$git\" &&\n ...\n\t) &&\n rm -fr \"$git\"\n"},{"id":"167648","messageId":"7vei46gm8v.fsf@alter.siamese.dyndns.org","threadId":"27317","inReplyTo":"1305055653-5133-2-git-send-email-luke@diamand.org","subject":"Re: [PATCH] git-p4: warn if git authorship won't be retained","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T23:38:08Z","receivedAt":"2011-05-10T23:38:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n> If the git commits you are submitting contain changes made by\n> other people, the authorship will not be retained. Change git-p4\n> to warn of this and to note that --preserve-user can be used\n> to solve the problem (if you have suitable permissions).\n> The warning can be disabled.\n>\n> Add a test case and update documentation.\n>\n> Requested-by: Junio C Hamano \n> Helped-by: Pete Wyckoff \n> Signed-off-by: Luke Diamand \n\nHeh, I didn't really \"request\" it; I am not a git-p4 user and am in no\nposition to judge if such a feature is useful. If you felt it is a good\nthing to do, please take full the credit (no need to resend; I am not\ncomplaining).\n\nThanks.\n"},{"id":"167674","messageId":"BANLkTinn5E29gLr89srxypEx9aauiL4h7Q@mail.gmail.com","threadId":"27317","inReplyTo":"7vhb92i2yu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-11T07:50:13Z","receivedAt":"2011-05-11T07:50:13Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Tue, May 10, 2011 at 11:51 PM, Junio C Hamano wrote:\n\n\n\n\n>\n>> + ...\n>> +     p4_check_commit_author usernamefile3 alice &&\n>> +     cd \"$TRASH_DIRECTORY\" &&\n>> +     rm -rf \"$git\" && mkdir \"$git\"\n>> +'\n>\n> When the commands in the && chain fails after 'cd \"$git\"' near the top but\n> not before 'cd \"$TRASH_DIRECTORY\"' near the end, you would end up starting\n> the next test from somewhere different from \"$TRASH_DIRECTORY\".\n>\n> Do it like this instead:\n>\n>        \"$GITP4\" clone --dest=\"$git\" //depot &&\n>        (\n>                cd \"$git\" &&\n>                ...\n>        ) &&\n>        rm -fr \"$git\"\n>\n\nThanks - that might explain a few oddities I had when testing.\n\nThere's a few others of those in the earlier tests; I'll resend this\npatch, and send a second patch to fix those if that's ok.\n\nRegards!\nLuke\n"}]}