{"thread":{"id":"30951","subject":"[PATCH 0/3] git p4: notice Jobs: section in submit","startedAt":"2012-07-04T13:34:17Z","lastAt":"2012-07-14T13:53:07Z","messageCount":8,"participants":["Pete Wyckoff","Luke Diamand"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"194616","messageId":"1341408860-26965-1-git-send-email-pw@padd.com","threadId":"30951","inReplyTo":null,"subject":"[PATCH 0/3] git p4: notice Jobs: section in submit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-04T13:34:17Z","receivedAt":"2012-07-04T13:34:17Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Perforce has an idea of a \"job\" and can connect changes to jobs\nin order to connect code fixes to an external bug-tracking system,\nfor instance.  The job can be specified in a change by adding\na section that looks like:\n\n    Jobs:\n\tproject-6032\n\nFor code committed from git p4, it would be nice to use a similar\nnotation in the commit message and have the jobs wind up in the\nright section in the p4 change description.\n\nThis was discussed in\nhttp://thread.gmane.org/gmane.comp.version-control.git/200445 .\n\nThese commits are on origin/next, as they depend on changes\nin pw/git-p4-tests that was merged into next on Jul 3.\n\nPete Wyckoff (3):\n  git p4: remove unused P4Submit interactive setting\n  git p4 test: refactor marshal_dump\n  git p4: notice Jobs lines in git commit messages\n\n git-p4.py                | 188 ++++++++++++++++++++++++++---------------------\n t/lib-git-p4.sh          |  14 ++++\n t/t9800-git-p4-basic.sh  |   5 --\n t/t9807-git-p4-submit.sh | 155 ++++++++++++++++++++++++++++++++++++++\n 4 files changed, 274 insertions(+), 88 deletions(-)\n\n-- \n1.7.11.1.125.g4a65fea\n"},{"id":"194617","messageId":"1341408860-26965-2-git-send-email-pw@padd.com","threadId":"30951","inReplyTo":"1341408860-26965-1-git-send-email-pw@padd.com","subject":"[PATCH 1/3] git p4: remove unused P4Submit interactive setting","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-04T13:34:18Z","receivedAt":"2012-07-04T13:34:18Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The code is unused.  Delete.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 144 ++++++++++++++++++++++++++++----------------------------------\n 1 file changed, 66 insertions(+), 78 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex f895a24..542c20a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):\n         ]\n         self.description = \"Submit changes from git to the perforce depot.\"\n         self.usage += \" [name of git branch to submit into perforce depot]\"\n-        self.interactive = True\n         self.origin = \"\"\n         self.detectRenames = False\n         self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n@@ -1209,86 +1208,77 @@ class P4Submit(Command, P4UserMap):\n \n         template = self.prepareSubmitTemplate()\n \n-        if self.interactive:\n-            submitTemplate = self.prepareLogMessage(template, logMessage)\n+        submitTemplate = self.prepareLogMessage(template, logMessage)\n \n-            if self.preserveUser:\n-               submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\n-\n-            if os.environ.has_key(\"P4DIFF\"):\n-                del(os.environ[\"P4DIFF\"])\n-            diff = \"\"\n-            for editedFile in editedFiles:\n-                diff += p4_read_pipe(['diff', '-du',\n-                                      wildcard_encode(editedFile)])\n-\n-            newdiff = \"\"\n-            for newFile in filesToAdd:\n-                newdiff += \"==== new file ====\\n\"\n-                newdiff += \"--- /dev/null\\n\"\n-                newdiff += \"+++ %s\\n\" % newFile\n-                f = open(newFile, \"r\")\n-                for line in f.readlines():\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 option --preserve-user to modify authorship.\\n\"\n-                submitTemplate += \"######## Variable 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-            tmpFile = os.fdopen(handle, \"w+\")\n-            if self.isWindows:\n-                submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n-                separatorLine = separatorLine.replace(\"\\n\", \"\\r\\n\")\n-                newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n-            tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n+        if self.preserveUser:\n+           submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\n+\n+        if os.environ.has_key(\"P4DIFF\"):\n+            del(os.environ[\"P4DIFF\"])\n+        diff = \"\"\n+        for editedFile in editedFiles:\n+            diff += p4_read_pipe(['diff', '-du',\n+                                  wildcard_encode(editedFile)])\n+\n+        newdiff = \"\"\n+        for newFile in filesToAdd:\n+            newdiff += \"==== new file ====\\n\"\n+            newdiff += \"--- /dev/null\\n\"\n+            newdiff += \"+++ %s\\n\" % newFile\n+            f = open(newFile, \"r\")\n+            for line in f.readlines():\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 option --preserve-user to modify authorship.\\n\"\n+            submitTemplate += \"######## Variable 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+        tmpFile = os.fdopen(handle, \"w+\")\n+        if self.isWindows:\n+            submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n+            separatorLine = separatorLine.replace(\"\\n\", \"\\r\\n\")\n+            newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n+        tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n+        tmpFile.close()\n+\n+        if self.edit_template(fileName):\n+            # read the edited message and submit\n+            tmpFile = open(fileName, \"rb\")\n+            message = tmpFile.read()\n             tmpFile.close()\n+            submitTemplate = message[:message.index(separatorLine)]\n+            if self.isWindows:\n+                submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n+            p4_write_pipe(['submit', '-i'], submitTemplate)\n \n-            if self.edit_template(fileName):\n-                # read the edited message and submit\n-                tmpFile = open(fileName, \"rb\")\n-                message = tmpFile.read()\n-                tmpFile.close()\n-                submitTemplate = message[:message.index(separatorLine)]\n-                if self.isWindows:\n-                    submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n-                p4_write_pipe(['submit', '-i'], submitTemplate)\n-\n-                if self.preserveUser:\n-                    if p4User:\n-                        # Get last changelist number. Cannot easily get it from\n-                        # the submit command output as the output is\n-                        # unmarshalled.\n-                        changelist = self.lastP4Changelist()\n-                        self.modifyChangelistUser(changelist, p4User)\n-\n-                # The rename/copy happened by applying a patch that created a\n-                # new file.  This leaves it writable, which confuses p4.\n-                for f in pureRenameCopy:\n-                    p4_sync(f, \"-f\")\n-\n-            else:\n-                # skip this patch\n-                print \"Submission cancelled, undoing p4 changes.\"\n-                for f in editedFiles:\n-                    p4_revert(f)\n-                for f in filesToAdd:\n-                    p4_revert(f)\n-                    os.remove(f)\n+            if self.preserveUser:\n+                if p4User:\n+                    # Get last changelist number. Cannot easily get it from\n+                    # the submit command output as the output is\n+                    # unmarshalled.\n+                    changelist = self.lastP4Changelist()\n+                    self.modifyChangelistUser(changelist, p4User)\n+\n+            # The rename/copy happened by applying a patch that created a\n+            # new file.  This leaves it writable, which confuses p4.\n+            for f in pureRenameCopy:\n+                p4_sync(f, \"-f\")\n \n-            os.remove(fileName)\n         else:\n-            fileName = \"submit.txt\"\n-            file = open(fileName, \"w+\")\n-            file.write(self.prepareLogMessage(template, logMessage))\n-            file.close()\n-            print (\"Perforce submit template written as %s. \"\n-                   + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n-                   % (fileName, fileName))\n+            # skip this patch\n+            print \"Submission cancelled, undoing p4 changes.\"\n+            for f in editedFiles:\n+                p4_revert(f)\n+            for f in filesToAdd:\n+                p4_revert(f)\n+                os.remove(f)\n+\n+        os.remove(fileName)\n \n     # Export git tags as p4 labels. Create a p4 label and then tag\n     # with that.\n@@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):\n             commit = commits[0]\n             commits = commits[1:]\n             self.applyCommit(commit)\n-            if not self.interactive:\n-                break\n \n         if len(commits) == 0:\n             print \"All changes applied!\"\n-- \n1.7.11.1.125.g4a65fea\n"},{"id":"194618","messageId":"1341408860-26965-3-git-send-email-pw@padd.com","threadId":"30951","inReplyTo":"1341408860-26965-1-git-send-email-pw@padd.com","subject":"[PATCH 2/3] git p4 test: refactor marshal_dump","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-04T13:34:19Z","receivedAt":"2012-07-04T13:34:19Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This function will be useful in future tests.  Move it to\nthe git-p4 test library.  Let it accept an optional argument\nto pick a certain marshaled object out of the input stream.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n t/lib-git-p4.sh         | 14 ++++++++++++++\n t/t9800-git-p4-basic.sh |  5 -----\n 2 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 31d75ae..080b2c1 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -102,3 +102,17 @@ cleanup_git() {\n \trm -rf \"$git\" &&\n \tmkdir \"$git\"\n }\n+\n+marshal_dump() {\n+\twhat=$1 &&\n+\tline=${2:-1} &&\n+\tcat >\"$TRASH_DIRECTORY/marshal-dump.py\" <<-EOF &&\n+\timport marshal\n+\timport sys\n+\tfor i in range($line):\n+\t    d = marshal.load(sys.stdin)\n+\tprint d['$what']\n+\tEOF\n+\t\"$PYTHON_PATH\" \"$TRASH_DIRECTORY/marshal-dump.py\"\n+}\n+\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 07c2e15..b7ad716 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -155,11 +155,6 @@ test_expect_success 'clone bare' '\n \t)\n '\n \n-marshal_dump() {\n-\twhat=$1\n-\t\"$PYTHON_PATH\" -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-- \n1.7.11.1.125.g4a65fea\n"},{"id":"194619","messageId":"1341408860-26965-4-git-send-email-pw@padd.com","threadId":"30951","inReplyTo":"1341408860-26965-1-git-send-email-pw@padd.com","subject":"[PATCH 3/3] git p4: notice Jobs lines in git commit messages","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-04T13:34:20Z","receivedAt":"2012-07-04T13:34:20Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"P4 has a feature called \"jobs\" that allows linking changes\nto a bug tracking system or other tasks.  When submitting\ncode, a job name can be specified to mark that this change\nis associated with a particular job.\n\nTeach git-p4 to find an optional \"Jobs:\" line in git commit\nmessages and use them to make a Jobs section in the p4\nchange specifitation.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                |  46 ++++++++++++--\n t/t9807-git-p4-submit.sh | 155 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 195 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 542c20a..fa44817 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -854,9 +854,34 @@ class P4Submit(Command, P4UserMap):\n         if len(p4CmdList(\"opened ...\")) > 0:\n             die(\"You have files opened with perforce! Close them before starting the sync.\")\n \n-    # replaces everything between 'Description:' and the next P4 submit template field with the\n-    # commit message\n-    def prepareLogMessage(self, template, message):\n+    def separate_jobs_from_description(self, message):\n+        \"\"\"Extract and return a possible Jobs field in the commit\n+           message.  It goes into a separate section in the p4 change\n+           specification.\n+\n+           A jobs line starts with \"Jobs:\" and looks like a new field\n+           in a form.  Values are white-space separated on the same\n+           line or on following lines that start with a tab.\n+\n+           This does not parse and extract the full git commit message\n+           like a p4 form.  It just sees the Jobs: line as a marker\n+           to pass everything from then on directly into the p4 form,\n+           but outside the description section.\n+           \n+           Return a tuple (stripped log message, jobs string).\"\"\"\n+\n+        m = re.search(r'^Jobs:', message, re.MULTILINE)\n+        if m is None:\n+            return (message, None)\n+\n+        jobtext = message[m.start():]\n+        stripped_message = message[:m.start()].rstrip()\n+        return (stripped_message, jobtext)\n+\n+    def prepareLogMessage(self, template, message, jobs):\n+        \"\"\"Edits the template returned from \"p4 change -o\" to insert\n+           the message in the Description field, and the jobs text in\n+           the Jobs field.\"\"\"\n         result = \"\"\n \n         inDescriptionSection = False\n@@ -869,6 +894,9 @@ class P4Submit(Command, P4UserMap):\n             if inDescriptionSection:\n                 if line.startswith(\"Files:\") or line.startswith(\"Jobs:\"):\n                     inDescriptionSection = False\n+                    # insert Jobs section\n+                    if jobs:\n+                        result += jobs + \"\\n\"\n                 else:\n                     continue\n             else:\n@@ -980,7 +1008,13 @@ class P4Submit(Command, P4UserMap):\n         return 0\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+        \"\"\"Run \"p4 change -o\" to grab a change specification template.\n+           This does not use \"p4 -G\", as it is nice to keep the submission\n+           template in original order, since a human might edit it.\n+\n+           Remove lines in the Files section that show changes to files\n+           outside the depot path we're committing into.\"\"\"\n+\n         template = \"\"\n         inFilesSection = False\n         for line in p4_read_pipe_lines(['change', '-o']):\n@@ -1205,10 +1239,10 @@ class P4Submit(Command, P4UserMap):\n \n         logMessage = extractLogMessageFromGitCommit(id)\n         logMessage = logMessage.strip()\n+        (logMessage, jobs) = self.separate_jobs_from_description(logMessage)\n \n         template = self.prepareSubmitTemplate()\n-\n-        submitTemplate = self.prepareLogMessage(template, logMessage)\n+        submitTemplate = self.prepareLogMessage(template, logMessage, jobs)\n \n         if self.preserveUser:\n            submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex f23b4c3..d6d588c 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -182,6 +182,161 @@ test_expect_success 'submit rename' '\n \t)\n '\n \n+#\n+# Converting git commit message to p4 change description, including\n+# parsing out the optional Jobs: line.\n+#\n+test_expect_success 'simple one-line description' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo desc2 >desc2 &&\n+\t\tgit add desc2 &&\n+\t\tcat >msg <<-EOF &&\n+\t\tOne-line description line for desc2.\n+\t\tEOF\n+\t\tgit commit -F - <msg &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit &&\n+\t\tchange=$(p4 -G changes -m 1 //depot/... | \\\n+\t\t         marshal_dump change) &&\n+\t\t# marshal_dump always adds a newline\n+\t\tp4 -G describe $change | marshal_dump desc | sed \\$d >pmsg &&\n+\t\ttest_cmp msg pmsg\n+\t)\n+'\n+\n+test_expect_success 'description with odd formatting' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo desc3 >desc3 &&\n+\t\tgit add desc3 &&\n+\t\t(\n+\t\t\tprintf \"subject line\\n\\n\\tExtra tab\\nline.\\n\\n\" &&\n+\t\t\tprintf \"Description:\\n\\tBogus description marker\\n\\n\" &&\n+\t\t\t# git commit eats trailing newlines; only use one\n+\t\t\tprintf \"Files:\\n\\tBogus descs marker\\n\"\n+\t\t) >msg &&\n+\t\tgit commit -F - <msg &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit &&\n+\t\tchange=$(p4 -G changes -m 1 //depot/... | \\\n+\t\t         marshal_dump change) &&\n+\t\t# marshal_dump always adds a newline\n+\t\tp4 -G describe $change | marshal_dump desc | sed \\$d >pmsg &&\n+\t\ttest_cmp msg pmsg\n+\t)\n+'\n+\n+make_job() {\n+\tname=\"$1\" &&\n+\ttab=\"$(printf \\\\t)\" &&\n+\tp4 job -o | \\\n+\tsed -e \"/^Job:/s/.*/Job: $name/\" \\\n+\t    -e \"/^Description/{ n; s/.*/$tab job text/; }\" | \\\n+\tp4 job -i\n+}\n+\n+test_expect_success 'description with Jobs section at end' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo desc4 >desc4 &&\n+\t\tgit add desc4 &&\n+\t\techo 6060842 >jobname &&\n+\t\t(\n+\t\t\tprintf \"subject line\\n\\n\\tExtra tab\\nline.\\n\\n\" &&\n+\t\t\tprintf \"Files:\\n\\tBogus files marker\\n\" &&\n+\t\t\tprintf \"Junk: 3164175\\n\" &&\n+\t\t\tprintf \"Jobs: $(cat jobname)\\n\"\n+\t\t) >msg &&\n+\t\tgit commit -F - <msg &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# build a job\n+\t\tmake_job $(cat jobname) &&\n+\t\tgit p4 submit &&\n+\t\tchange=$(p4 -G changes -m 1 //depot/... | \\\n+\t\t         marshal_dump change) &&\n+\t\t# marshal_dump always adds a newline\n+\t\tp4 -G describe $change | marshal_dump desc | sed \\$d >pmsg &&\n+\t\t# make sure Jobs line and all following is gone\n+\t\tsed \"/^Jobs:/,\\$d\" msg >jmsg &&\n+\t\ttest_cmp jmsg pmsg &&\n+\t\t# make sure p4 knows about job\n+\t\tp4 -G describe $change | marshal_dump job0 >job0 &&\n+\t\ttest_cmp jobname job0\n+\t)\n+'\n+\n+test_expect_success 'description with Jobs and values on separate lines' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo desc5 >desc5 &&\n+\t\tgit add desc5 &&\n+\t\techo PROJ-6060842 >jobname1 &&\n+\t\techo PROJ-6060847 >jobname2 &&\n+\t\t(\n+\t\t\tprintf \"subject line\\n\\n\\tExtra tab\\nline.\\n\\n\" &&\n+\t\t\tprintf \"Files:\\n\\tBogus files marker\\n\" &&\n+\t\t\tprintf \"Junk: 3164175\\n\" &&\n+\t\t\tprintf \"Jobs:\\n\" &&\n+\t\t\tprintf \"\\t$(cat jobname1)\\n\" &&\n+\t\t\tprintf \"\\t$(cat jobname2)\\n\"\n+\t\t) >msg &&\n+\t\tgit commit -F - <msg &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# build two jobs\n+\t\tmake_job $(cat jobname1) &&\n+\t\tmake_job $(cat jobname2) &&\n+\t\tgit p4 submit &&\n+\t\tchange=$(p4 -G changes -m 1 //depot/... | \\\n+\t\t         marshal_dump change) &&\n+\t\t# marshal_dump always adds a newline\n+\t\tp4 -G describe $change | marshal_dump desc | sed \\$d >pmsg &&\n+\t\t# make sure Jobs line and all following is gone\n+\t\tsed \"/^Jobs:/,\\$d\" msg >jmsg &&\n+\t\ttest_cmp jmsg pmsg &&\n+\t\t# make sure p4 knows about the two jobs\n+\t\tp4 -G describe $change >change &&\n+\t\t(\n+\t\t\tmarshal_dump job0 <change &&\n+\t\t\tmarshal_dump job1 <change\n+\t\t) | sort >jobs &&\n+\t\tcat jobname1 jobname2 | sort >expected &&\n+\t\ttest_cmp expected jobs\n+\t)\n+'\n+\n+test_expect_success 'description with Jobs section and bogus following text' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo desc6 >desc6 &&\n+\t\tgit add desc6 &&\n+\t\techo 6060843 >jobname &&\n+\t\t(\n+\t\t\tprintf \"subject line\\n\\n\\tExtra tab\\nline.\\n\\n\" &&\n+\t\t\tprintf \"Files:\\n\\tBogus files marker\\n\" &&\n+\t\t\tprintf \"Junk: 3164175\\n\" &&\n+\t\t\tprintf \"Jobs: $(cat jobname)\\n\" &&\n+\t\t\tprintf \"MoreJunk: 3711\\n\"\n+\t\t) >msg &&\n+\t\tgit commit -F - <msg &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# build a job\n+\t\tmake_job $(cat jobname) &&\n+\t\ttest_must_fail git p4 submit 2>err &&\n+\t\ttest_i18ngrep \"Unknown field name\" err\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.11.1.125.g4a65fea\n"},{"id":"194623","messageId":"20120704134918.GA28164@padd.com","threadId":"30951","inReplyTo":"1341408860-26965-2-git-send-email-pw@padd.com","subject":"Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-04T13:49:18Z","receivedAt":"2012-07-04T13:49:18Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"The \"diff -w\" version looks like this.  A bit easier to review.\n\n--->8---\n\n>From 3327945176be35081f2c86bc40b294ed9f73ea9a Mon Sep 17 00:00:00 2001\nFrom: Pete Wyckoff <pw@padd.com>\nDate: Sun, 24 Jun 2012 15:39:18 -0400\nSubject: [PATCH] git p4: remove unused P4Submit interactive setting\n\nThe code is unused.  Delete.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py | 12 ------------\n 1 file changed, 12 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex f895a24..542c20a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):\n         ]\n         self.description = \"Submit changes from git to the perforce depot.\"\n         self.usage += \" [name of git branch to submit into perforce depot]\"\n-        self.interactive = True\n         self.origin = \"\"\n         self.detectRenames = False\n         self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n@@ -1209,7 +1208,6 @@ class P4Submit(Command, P4UserMap):\n \n         template = self.prepareSubmitTemplate()\n \n-        if self.interactive:\n         submitTemplate = self.prepareLogMessage(template, logMessage)\n \n         if self.preserveUser:\n@@ -1281,14 +1279,6 @@ class P4Submit(Command, P4UserMap):\n                 os.remove(f)\n \n         os.remove(fileName)\n-        else:\n-            fileName = \"submit.txt\"\n-            file = open(fileName, \"w+\")\n-            file.write(self.prepareLogMessage(template, logMessage))\n-            file.close()\n-            print (\"Perforce submit template written as %s. \"\n-                   + \"Please review/edit and then use p4 submit -i < %s to submit directly!\"\n-                   % (fileName, fileName))\n \n     # Export git tags as p4 labels. Create a p4 label and then tag\n     # with that.\n@@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):\n             commit = commits[0]\n             commits = commits[1:]\n             self.applyCommit(commit)\n-            if not self.interactive:\n-                break\n \n         if len(commits) == 0:\n             print \"All changes applied!\"\n-- \n1.7.11.1.125.g4a65fea\n"},{"id":"194632","messageId":"4FF54041.2000507@diamand.org","threadId":"30951","inReplyTo":"1341408860-26965-2-git-send-email-pw@padd.com","subject":"Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-07-05T07:20:33Z","receivedAt":"2012-07-05T07:20:33Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 04/07/12 14:34, Pete Wyckoff wrote:\n> The code is unused.  Delete.\n\nI've used that non-interactive code path in the past, in the very early \ndays of using it (setting interactive to false manually).\n\nThe nice thing about it is that if you're using git-p4 for the very \nfirst time it lets you do the final submission to p4 by hand, without \nhaving to trust the script to do the right thing. Once I convinced \nmyself that git-p4 was doing the right thing, I then stopped using it.\n\nIs it worth retaining, perhaps fixed so that it can be set on the \ncommand line and documented? Or just discard?\n\nThanks\nLuke\n\n>\n> Signed-off-by: Pete Wyckoff<pw@padd.com>\n> ---\n>   git-p4.py | 144 ++++++++++++++++++++++++++++----------------------------------\n>   1 file changed, 66 insertions(+), 78 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index f895a24..542c20a 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):\n>           ]\n>           self.description = \"Submit changes from git to the perforce depot.\"\n>           self.usage += \" [name of git branch to submit into perforce depot]\"\n> -        self.interactive = True\n>           self.origin = \"\"\n>           self.detectRenames = False\n>           self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n> @@ -1209,86 +1208,77 @@ class P4Submit(Command, P4UserMap):\n>\n>           template = self.prepareSubmitTemplate()\n>\n> -        if self.interactive:\n> -            submitTemplate = self.prepareLogMessage(template, logMessage)\n> +        submitTemplate = self.prepareLogMessage(template, logMessage)\n>\n> -            if self.preserveUser:\n> -               submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\n> -\n> -            if os.environ.has_key(\"P4DIFF\"):\n> -                del(os.environ[\"P4DIFF\"])\n> -            diff = \"\"\n> -            for editedFile in editedFiles:\n> -                diff += p4_read_pipe(['diff', '-du',\n> -                                      wildcard_encode(editedFile)])\n> -\n> -            newdiff = \"\"\n> -            for newFile in filesToAdd:\n> -                newdiff += \"==== new file ====\\n\"\n> -                newdiff += \"--- /dev/null\\n\"\n> -                newdiff += \"+++ %s\\n\" % newFile\n> -                f = open(newFile, \"r\")\n> -                for line in f.readlines():\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 option --preserve-user to modify authorship.\\n\"\n> -                submitTemplate += \"######## Variable 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> -            tmpFile = os.fdopen(handle, \"w+\")\n> -            if self.isWindows:\n> -                submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n> -                separatorLine = separatorLine.replace(\"\\n\", \"\\r\\n\")\n> -                newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n> -            tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n> +        if self.preserveUser:\n> +           submitTemplate = submitTemplate + (\"\\n######## Actual user %s, modified after commit\\n\" % p4User)\n> +\n> +        if os.environ.has_key(\"P4DIFF\"):\n> +            del(os.environ[\"P4DIFF\"])\n> +        diff = \"\"\n> +        for editedFile in editedFiles:\n> +            diff += p4_read_pipe(['diff', '-du',\n> +                                  wildcard_encode(editedFile)])\n> +\n> +        newdiff = \"\"\n> +        for newFile in filesToAdd:\n> +            newdiff += \"==== new file ====\\n\"\n> +            newdiff += \"--- /dev/null\\n\"\n> +            newdiff += \"+++ %s\\n\" % newFile\n> +            f = open(newFile, \"r\")\n> +            for line in f.readlines():\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 option --preserve-user to modify authorship.\\n\"\n> +            submitTemplate += \"######## Variable 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> +        tmpFile = os.fdopen(handle, \"w+\")\n> +        if self.isWindows:\n> +            submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n> +            separatorLine = separatorLine.replace(\"\\n\", \"\\r\\n\")\n> +            newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n> +        tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n> +        tmpFile.close()\n> +\n> +        if self.edit_template(fileName):\n> +            # read the edited message and submit\n> +            tmpFile = open(fileName, \"rb\")\n> +            message = tmpFile.read()\n>               tmpFile.close()\n> +            submitTemplate = message[:message.index(separatorLine)]\n> +            if self.isWindows:\n> +                submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n> +            p4_write_pipe(['submit', '-i'], submitTemplate)\n>\n> -            if self.edit_template(fileName):\n> -                # read the edited message and submit\n> -                tmpFile = open(fileName, \"rb\")\n> -                message = tmpFile.read()\n> -                tmpFile.close()\n> -                submitTemplate = message[:message.index(separatorLine)]\n> -                if self.isWindows:\n> -                    submitTemplate = submitTemplate.replace(\"\\r\\n\", \"\\n\")\n> -                p4_write_pipe(['submit', '-i'], submitTemplate)\n> -\n> -                if self.preserveUser:\n> -                    if p4User:\n> -                        # Get last changelist number. Cannot easily get it from\n> -                        # the submit command output as the output is\n> -                        # unmarshalled.\n> -                        changelist = self.lastP4Changelist()\n> -                        self.modifyChangelistUser(changelist, p4User)\n> -\n> -                # The rename/copy happened by applying a patch that created a\n> -                # new file.  This leaves it writable, which confuses p4.\n> -                for f in pureRenameCopy:\n> -                    p4_sync(f, \"-f\")\n> -\n> -            else:\n> -                # skip this patch\n> -                print \"Submission cancelled, undoing p4 changes.\"\n> -                for f in editedFiles:\n> -                    p4_revert(f)\n> -                for f in filesToAdd:\n> -                    p4_revert(f)\n> -                    os.remove(f)\n> +            if self.preserveUser:\n> +                if p4User:\n> +                    # Get last changelist number. Cannot easily get it from\n> +                    # the submit command output as the output is\n> +                    # unmarshalled.\n> +                    changelist = self.lastP4Changelist()\n> +                    self.modifyChangelistUser(changelist, p4User)\n> +\n> +            # The rename/copy happened by applying a patch that created a\n> +            # new file.  This leaves it writable, which confuses p4.\n> +            for f in pureRenameCopy:\n> +                p4_sync(f, \"-f\")\n>\n> -            os.remove(fileName)\n>           else:\n> -            fileName = \"submit.txt\"\n> -            file = open(fileName, \"w+\")\n> -            file.write(self.prepareLogMessage(template, logMessage))\n> -            file.close()\n> -            print (\"Perforce submit template written as %s. \"\n> -                   + \"Please review/edit and then use p4 submit -i<  %s to submit directly!\"\n> -                   % (fileName, fileName))\n> +            # skip this patch\n> +            print \"Submission cancelled, undoing p4 changes.\"\n> +            for f in editedFiles:\n> +                p4_revert(f)\n> +            for f in filesToAdd:\n> +                p4_revert(f)\n> +                os.remove(f)\n> +\n> +        os.remove(fileName)\n>\n>       # Export git tags as p4 labels. Create a p4 label and then tag\n>       # with that.\n> @@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):\n>               commit = commits[0]\n>               commits = commits[1:]\n>               self.applyCommit(commit)\n> -            if not self.interactive:\n> -                break\n>\n>           if len(commits) == 0:\n>               print \"All changes applied!\"\n"},{"id":"194652","messageId":"20120705123010.GA31388@padd.com","threadId":"30951","inReplyTo":"4FF54041.2000507@diamand.org","subject":"Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-05T12:30:10Z","receivedAt":"2012-07-05T12:30:10Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Thu, 05 Jul 2012 08:20 +0100:\n> On 04/07/12 14:34, Pete Wyckoff wrote:\n> >The code is unused.  Delete.\n> \n> I've used that non-interactive code path in the past, in the very\n> early days of using it (setting interactive to false manually).\n> \n> The nice thing about it is that if you're using git-p4 for the very\n> first time it lets you do the final submission to p4 by hand,\n> without having to trust the script to do the right thing. Once I\n> convinced myself that git-p4 was doing the right thing, I then\n> stopped using it.\n> \n> Is it worth retaining, perhaps fixed so that it can be set on the\n> command line and documented? Or just discard?\n\nMy biggest complaint is that there's no way to enable the option.\nYou have to edit the code to change self.interactive to False, as\nyou pointed out.\n\nThen it doesn't help you with the submit message, and doesn't\ndo the little details of cleaning up pure-copied files or\nchanging the username for preserveUser.\n\nWhat you're doing makes sense, though, but maybe there's a\ncleaner way to provide that functionality.\n\nWe could build the change then say \"type p4 submit -c ... if it\nlooks good\".  Still doesn't handle the little details.\n\nWe could spawn a shell to let them go inspect.\n\nWe could try to implement a \"--continue\" option, and give them\na chance to edit.\n\nI've got an upcoming series that changes the interaction loop on\nconflict, and makes it easier to do some interaction at each\npatch, possibly before applying too.  Might make things easier.\n\n\t\t-- Pete\n"},{"id":"195046","messageId":"20120714135307.GA27609@padd.com","threadId":"30951","inReplyTo":"20120705123010.GA31388@padd.com","subject":"Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-07-14T13:53:07Z","receivedAt":"2012-07-14T13:53:07Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"pw@padd.com wrote on Thu, 05 Jul 2012 08:30 -0400:\n> luke@diamand.org wrote on Thu, 05 Jul 2012 08:20 +0100:\n> > On 04/07/12 14:34, Pete Wyckoff wrote:\n> > >The code is unused.  Delete.\n> > \n> > I've used that non-interactive code path in the past, in the very\n> > early days of using it (setting interactive to false manually).\n> > \n> > The nice thing about it is that if you're using git-p4 for the very\n> > first time it lets you do the final submission to p4 by hand,\n> > without having to trust the script to do the right thing. Once I\n> > convinced myself that git-p4 was doing the right thing, I then\n> > stopped using it.\n> > \n> > Is it worth retaining, perhaps fixed so that it can be set on the\n> > command line and documented? Or just discard?\n> \n> My biggest complaint is that there's no way to enable the option.\n> You have to edit the code to change self.interactive to False, as\n> you pointed out.\n> \n> Then it doesn't help you with the submit message, and doesn't\n> do the little details of cleaning up pure-copied files or\n> changing the username for preserveUser.\n> \n> What you're doing makes sense, though, but maybe there's a\n> cleaner way to provide that functionality.\n> \n> We could build the change then say \"type p4 submit -c ... if it\n> looks good\".  Still doesn't handle the little details.\n> \n> We could spawn a shell to let them go inspect.\n> \n> We could try to implement a \"--continue\" option, and give them\n> a chance to edit.\n> \n> I've got an upcoming series that changes the interaction loop on\n> conflict, and makes it easier to do some interaction at each\n> patch, possibly before applying too.  Might make things easier.\n\nI did code up two new options to \"git p4 submit\":\n\n    --dry-run : just show the commits that would be submitted\n\n    --prepare-p4-only : open/add, apply patch, but do not submit\n\nThe latter prints a rather lengthy message about how to submit\nor revert the changes.  It fills the role of self.interactive,\nhopefully.\n\nI'll send the patches out for review once the other in-flight\nchanges have settled.\n\n\t\t-- Pete\n"}]}