{"thread":{"id":"27347","subject":"[PATCH/RFC v6] git-p4: warn if git authorship won't be retained","startedAt":"2011-05-13T19:45:59Z","lastAt":"2011-05-13T19:46:00Z","messageCount":2,"participants":["Luke Diamand"],"isPatch":true,"patchVersion":6,"patchTotal":null},"messages":[{"id":"167827","messageId":"1305315960-17818-1-git-send-email-luke@diamand.org","threadId":"27347","inReplyTo":null,"subject":"[PATCH/RFC v6] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-13T19:45:59Z","receivedAt":"2011-05-13T19:45:59Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Updated version of authorship warning change, with modifications to\ntest harness as compared to v5 suggested by Junio.\n\nJunio - do you need the same fixes to the test cases I put in earlier\n(in 3ea2cfd488ed70e03d8d6f10e601784926db8008)? The redirects have\nextra whitespace. If so, how should I best do that?\n\nThanks!\nLuke\n\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              |   41 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 79 insertions(+), 2 deletions(-)\n"},{"id":"167828","messageId":"1305315960-17818-2-git-send-email-luke@diamand.org","threadId":"27347","inReplyTo":"1305315960-17818-1-git-send-email-luke@diamand.org","subject":"[PATCH/RFC v6] git-p4: warn if git authorship won't be retained","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-05-13T19:46:00Z","receivedAt":"2011-05-13T19:46:00Z","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\nSigned-off-by: Luke Diamand <luke@diamand.org>\n---\n contrib/fast-import/git-p4     |   33 ++++++++++++++++++++++++++++++-\n contrib/fast-import/git-p4.txt |    7 ++++++\n t/t9800-git-p4.sh              |   41 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 79 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..33b0127 100755\n--- a/t/t9800-git-p4.sh\n+++ b/t/t9800-git-p4.sh\n@@ -160,6 +160,13 @@ p4_check_commit_author() {\n     fi\n }\n \n+make_change_by_user() {\n+\tfile=$1 name=$2 email=$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 +220,40 @@ 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+\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+'\n+\n+\n test_expect_success 'shutdown' '\n \tpid=`pgrep -f p4d` &&\n \ttest -n \"$pid\" &&\n-- \n1.7.1\n"}]}