{"thread":{"id":"35634","subject":"[PATCH] git-p4: Do not include diff in spec file when just preparing p4","startedAt":"2014-01-10T18:18:07Z","lastAt":"2014-06-11T13:36:02Z","messageCount":13,"participants":["Maxime Coste","Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"233001","messageId":"20140110181807.GA29164@nekage","threadId":"35634","inReplyTo":null,"subject":"[PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-01-10T18:18:07Z","receivedAt":"2014-01-10T18:18:07Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"The diff information render the spec file unusable as is by p4,\ndo not include it when run with --prepare-p4-only so that the\ngiven file can be directly passed to p4.\n---\n git-p4.py | 70 +++++++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 39 insertions(+), 31 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5ea8bb8..7c65340 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1397,38 +1397,14 @@ class P4Submit(Command, P4UserMap):\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-        # diff\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-        # new file diff\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-        # change description file: submitTemplate, separatorLine, diff, newdiff\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.prepare_p4_only:\n+            (handle, fileName) = tempfile.mkstemp()\n+            tmpFile = os.fdopen(handle, \"w+\")\n+            if self.isWindows:\n+                submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n+            tmpFile.write(submitTemplate)\n+            tmpFile.close()\n+\n             #\n             # Leave the p4 tree prepared, and the submit template around\n             # and let the user decide what to do next\n@@ -1463,6 +1439,38 @@ class P4Submit(Command, P4UserMap):\n             print\n             return True\n \n+        else:\n+            separatorLine = \"######## everything below this line is just the diff #######\\n\"\n+\n+            # diff\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+            # new file diff\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+            # change description file: submitTemplate, separatorLine, diff, newdiff\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         #\n         # Let the user edit the change description, then submit it.\n         #\n-- \n1.8.5.2\n"},{"id":"233045","messageId":"20140112222946.GA13519@padd.com","threadId":"35634","inReplyTo":"20140110181807.GA29164@nekage","subject":"Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2014-01-12T22:29:46Z","receivedAt":"2014-01-12T22:29:46Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"frrrwww@gmail.com wrote on Fri, 10 Jan 2014 18:18 +0000:\n> The diff information render the spec file unusable as is by p4,\n> do not include it when run with --prepare-p4-only so that the\n> given file can be directly passed to p4.\n\nThanks for the patch, but I'm curious how you'd like this to\nwork.  I never use the option myself.\n\nAs it is, --prepare-p4-only generates a file in /tmp/ that has\nexactly the contents you'd see in the editor during \"git p4\nsubmit\".  It includes the diff of the change, presumably to help\nwith writing the description.\n\nNow you can't actually feed this file directly to \"p4 submit\"\nwithout deleting the diff.  That's the part you don't like?\n\n\t\t-- Pete\n"},{"id":"233051","messageId":"20140113121011.GA9711@nekage","threadId":"35634","inReplyTo":"20140112222946.GA13519@padd.com","subject":"Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-01-13T12:10:11Z","receivedAt":"2014-01-13T12:10:11Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"Hello,\n\nOn Sun, Jan 12, 2014 at 05:29:46PM -0500, Pete Wyckoff wrote:\n> Thanks for the patch, but I'm curious how you'd like this to\n> work.  I never use the option myself.\n> \n> As it is, --prepare-p4-only generates a file in /tmp/ that has\n> exactly the contents you'd see in the editor during \"git p4\n> submit\".  It includes the diff of the change, presumably to help\n> with writing the description.\n\nYes, I believe it makes sense to display the diff in this case, as we\ncan remove it later programmatically.\n \n> Now you can't actually feed this file directly to \"p4 submit\"\n> without deleting the diff.  That's the part you don't like?\n\nYes, I do not use that for submitting, but for shelving. I can run\ngit p4 submit --prepare-p4-only followed by p4 shelve -i < /tmp/...\nand perforce will shelve the corresponding change.\n\nRemoving the diff could be done externally, however git-p4 itself\ntells the user it can submit using the generated file, which is\nnot the case if we keep the diff in it.\n\nCheers,\n\nMaxime Coste.\n"},{"id":"233070","messageId":"20140114000613.GA11594@padd.com","threadId":"35634","inReplyTo":"20140113121011.GA9711@nekage","subject":"Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2014-01-14T00:06:13Z","receivedAt":"2014-01-14T00:06:13Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"frrrwww@gmail.com wrote on Mon, 13 Jan 2014 12:10 +0000:\n> Hello,\n> \n> On Sun, Jan 12, 2014 at 05:29:46PM -0500, Pete Wyckoff wrote:\n> > Thanks for the patch, but I'm curious how you'd like this to\n> > work.  I never use the option myself.\n> > \n> > As it is, --prepare-p4-only generates a file in /tmp/ that has\n> > exactly the contents you'd see in the editor during \"git p4\n> > submit\".  It includes the diff of the change, presumably to help\n> > with writing the description.\n> \n> Yes, I believe it makes sense to display the diff in this case, as we\n> can remove it later programmatically.\n>  \n> > Now you can't actually feed this file directly to \"p4 submit\"\n> > without deleting the diff.  That's the part you don't like?\n> \n> Yes, I do not use that for submitting, but for shelving. I can run\n> git p4 submit --prepare-p4-only followed by p4 shelve -i < /tmp/...\n> and perforce will shelve the corresponding change.\n> \n> Removing the diff could be done externally, however git-p4 itself\n> tells the user it can submit using the generated file, which is\n> not the case if we keep the diff in it.\n\nI'm convinced.  That explanation makes sense, thanks.\n\nIt would be nice to do a few more things with this patch.  Here's\nsome ideas, sorted in priority order.\n\n    1.  Put slightly more text into the commit message, possibly\n    from your email above.\n\n    2.  Refactor out that big chunk of code instead of just\n    moving it.  Selectively call it only if not prepare_p4_only.\n\n    3.  Modify the t9807 test 'submit --prepare-p4-only' to make\n    sure the diff isn't there.\n\n    4.  Documentation update?  Probably not necessary.\n\nLet me know if you're interested in doing any of this.\n\n\t\t-- Pete\n"},{"id":"242629","messageId":"20140524013942.GA29751@nekage","threadId":"35634","inReplyTo":"20140114000613.GA11594@padd.com","subject":"[PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-05-24T01:39:42Z","receivedAt":"2014-05-24T01:39:42Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"The diff information render the spec file unusable as is by p4,\ndo not include it when run with --prepare-p4-only so that the\ngiven file can be directly passed to p4.\n\nWith --prepare-p4-only, git-p4 already tells the user it can use\np4 submit with the generated spec file. This fails because of the\ndiff being present in the file. Not including the diff fixes that.\n\nWithout --prepare-p4-only, keeping the diff makes sense for a\nquick review of the patch before submitting it. And does not cause\nproblems with p4 as we remove it programmatically.\n\nSigned-off-by: Maxime Coste <frrrwww@gmail.com>\n---\n git-p4.py                | 49 +++++++++++++++++++++++++-----------------------\n t/t9807-git-p4-submit.sh |  3 ++-\n 2 files changed, 28 insertions(+), 24 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 773cafc..7bb0f73 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1238,6 +1238,28 @@ class P4Submit(Command, P4UserMap):\n             if response == 'n':\n                 return False\n \n+    def get_diff_description(self, editedFiles):\n+        # diff\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+        # new file diff\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+        return diff + newdiff\n+\n     def applyCommit(self, id):\n         \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n \n@@ -1398,34 +1420,15 @@ class P4Submit(Command, P4UserMap):\n             submitTemplate += \"######## Variable git-p4.skipUserNameCheck hides this message.\\n\"\n \n         separatorLine = \"######## everything below this line is just the diff #######\\n\"\n+        if not self.prepare_p4_only:\n+            submitTemplate += separatorLine\n+            submitTemplate += self.get_diff_description(editedFiles)\n \n-        # diff\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-        # new file diff\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-        # change description file: submitTemplate, separatorLine, diff, newdiff\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.write(submitTemplate)\n         tmpFile.close()\n \n         if self.prepare_p4_only:\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 4caf36e..7fab2ed 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -403,7 +403,8 @@ test_expect_success 'submit --prepare-p4-only' '\n \t\tgit commit -m \"prep only add\" &&\n \t\tgit p4 submit --prepare-p4-only >out &&\n \t\ttest_i18ngrep \"prepared for submission\" out &&\n-\t\ttest_i18ngrep \"must be deleted\" out\n+\t\ttest_i18ngrep \"must be deleted\" out &&\n+\t\t! test_i18ngrep \"everything below this line is just the diff\" out\n \t) &&\n \t(\n \t\tcd \"$cli\" &&\n-- \n1.9.3\n"},{"id":"242630","messageId":"20140524014441.GB29751@nekage","threadId":"35634","inReplyTo":"20140524013942.GA29751@nekage","subject":"Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-05-24T01:44:41Z","receivedAt":"2014-05-24T01:44:41Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"Hello\n\nSorry for the delay, I hope that version is more acceptable.\n\nI updated the test case as well, but did not manage to get the actual p4\ntests to work here (I have p4 and p4d installed, they start but all the\nother tests seems to fail). Still the change is straightforward.\n\nCheers,\n\nMaxime Coste.\n"},{"id":"242640","messageId":"20140524135215.GA9386@padd.com","threadId":"35634","inReplyTo":"20140524013942.GA29751@nekage","subject":"Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2014-05-24T13:52:15Z","receivedAt":"2014-05-24T13:52:15Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"frrrwww@gmail.com wrote on Sat, 24 May 2014 02:39 +0100:\n> The diff information render the spec file unusable as is by p4,\n> do not include it when run with --prepare-p4-only so that the\n> given file can be directly passed to p4.\n> \n> With --prepare-p4-only, git-p4 already tells the user it can use\n> p4 submit with the generated spec file. This fails because of the\n> diff being present in the file. Not including the diff fixes that.\n> \n> Without --prepare-p4-only, keeping the diff makes sense for a\n> quick review of the patch before submitting it. And does not cause\n> problems with p4 as we remove it programmatically.\n> \n> Signed-off-by: Maxime Coste <frrrwww@gmail.com>\n\nHi Maxime.  This looks really good.  Even the Windows section\nis fine; thanks for paying attention there too.\n\nI'm not particularly worried about having a new test for this.\nYour tweak to the existing 9807 is fine.  Unless of course you\nhave one ready to go.\n\nAcked-by: Pete Wyckoff <pw@padd.com>\n\nYou might add my ack and send it directly to Junio + CC the list.\nIt'll be a nice improvement for the next available release.\n\n\t\t-- Pete\n"},{"id":"242641","messageId":"20140524174034.GA7560@nekage","threadId":"35634","inReplyTo":"20140524135215.GA9386@padd.com","subject":"[PATCH] git-p4: Do not include diff in spec file when just preparing p4","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-05-24T17:40:35Z","receivedAt":"2014-05-24T17:40:35Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"The diff information render the spec file unusable as is by p4,\ndo not include it when run with --prepare-p4-only so that the\ngiven file can be directly passed to p4.\n\nWith --prepare-p4-only, git-p4 already tells the user it can use\np4 submit with the generated spec file. This fails because of the\ndiff being present in the file. Not including the diff fixes that.\n\nWithout --prepare-p4-only, keeping the diff makes sense for a\nquick review of the patch before submitting it. And does not cause\nproblems with p4 as we remove it programmatically.\n\nSigned-off-by: Maxime Coste <frrrwww@gmail.com>\nAcked-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py                | 49 +++++++++++++++++++++++++-----------------------\n t/t9807-git-p4-submit.sh |  3 ++-\n 2 files changed, 28 insertions(+), 24 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 773cafc..7bb0f73 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1238,6 +1238,28 @@ class P4Submit(Command, P4UserMap):\n             if response == 'n':\n                 return False\n \n+    def get_diff_description(self, editedFiles):\n+        # diff\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+        # new file diff\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+        return diff + newdiff\n+\n     def applyCommit(self, id):\n         \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n \n@@ -1398,34 +1420,15 @@ class P4Submit(Command, P4UserMap):\n             submitTemplate += \"######## Variable git-p4.skipUserNameCheck hides this message.\\n\"\n \n         separatorLine = \"######## everything below this line is just the diff #######\\n\"\n+        if not self.prepare_p4_only:\n+            submitTemplate += separatorLine\n+            submitTemplate += self.get_diff_description(editedFiles)\n \n-        # diff\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-        # new file diff\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-        # change description file: submitTemplate, separatorLine, diff, newdiff\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.write(submitTemplate)\n         tmpFile.close()\n \n         if self.prepare_p4_only:\ndiff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh\nindex 4caf36e..7fab2ed 100755\n--- a/t/t9807-git-p4-submit.sh\n+++ b/t/t9807-git-p4-submit.sh\n@@ -403,7 +403,8 @@ test_expect_success 'submit --prepare-p4-only' '\n \t\tgit commit -m \"prep only add\" &&\n \t\tgit p4 submit --prepare-p4-only >out &&\n \t\ttest_i18ngrep \"prepared for submission\" out &&\n-\t\ttest_i18ngrep \"must be deleted\" out\n+\t\ttest_i18ngrep \"must be deleted\" out &&\n+\t\t! test_i18ngrep \"everything below this line is just the diff\" out\n \t) &&\n \t(\n \t\tcd \"$cli\" &&\n-- \n1.9.3\n"},{"id":"243723","messageId":"20140610121446.GA25634@nekage","threadId":"35634","inReplyTo":"20140524174034.GA7560@nekage","subject":"[PATCH] Fix git-p4 submit in non --prepare-p4-only mode","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-06-10T12:14:46Z","receivedAt":"2014-06-10T12:14:46Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here\nis a proper fix, including proper handling for windows end of lines.\n\nSigned-off-by: Maxime Coste <frrrwww@gmail.com>\n---\n git-p4.py | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 7bb0f73..ff132b2 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):\n             if response == 'n':\n                 return False\n \n-    def get_diff_description(self, editedFiles):\n+    def get_diff_description(self, editedFiles, filesToAdd):\n         # diff\n         if os.environ.has_key(\"P4DIFF\"):\n             del(os.environ[\"P4DIFF\"])\n@@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):\n                 newdiff += \"+\" + line\n             f.close()\n \n-        return diff + newdiff\n+        return (diff + newdiff).replace('\\r\\n', '\\n')\n \n     def applyCommit(self, id):\n         \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n@@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):\n         separatorLine = \"######## everything below this line is just the diff #######\\n\"\n         if not self.prepare_p4_only:\n             submitTemplate += separatorLine\n-            submitTemplate += self.get_diff_description(editedFiles)\n+            submitTemplate += self.get_diff_description(editedFiles, filesToAdd)\n \n         (handle, fileName) = tempfile.mkstemp()\n-        tmpFile = os.fdopen(handle, \"w+\")\n+        tmpFile = os.fdopen(handle, \"w+b\")\n         if self.isWindows:\n             submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n         tmpFile.write(submitTemplate)\n@@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):\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+                message = message.replace(\"\\r\\n\", \"\\n\")\n+            submitTemplate = message[:message.index(separatorLine)]\n             p4_write_pipe(['submit', '-i'], submitTemplate)\n \n             if self.preserveUser:\n-- \n2.0.0\n"},{"id":"243859","messageId":"20140610223958.GA10049@padd.com","threadId":"35634","inReplyTo":"20140610121446.GA25634@nekage","subject":"Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2014-06-10T22:39:58Z","receivedAt":"2014-06-10T22:39:58Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"frrrwww@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:\n> b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here\n> is a proper fix, including proper handling for windows end of lines.\n\nI guess we don't have test coverage for these cases?  Is this\nsomething that should get put into a maintenance release, quickly?\n\nThe fix looks good.  It's surprising that none of the tests\nmanaged to add a file and trigger the failure.\n\nI'll ack this again, as it looks okay, but hope you ran all the\nunit tests successfully on your machine.\n\n\t\t-- Pete\n\n> Signed-off-by: Maxime Coste <frrrwww@gmail.com>\n> ---\n>  git-p4.py | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index 7bb0f73..ff132b2 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):\n>              if response == 'n':\n>                  return False\n>  \n> -    def get_diff_description(self, editedFiles):\n> +    def get_diff_description(self, editedFiles, filesToAdd):\n>          # diff\n>          if os.environ.has_key(\"P4DIFF\"):\n>              del(os.environ[\"P4DIFF\"])\n> @@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):\n>                  newdiff += \"+\" + line\n>              f.close()\n>  \n> -        return diff + newdiff\n> +        return (diff + newdiff).replace('\\r\\n', '\\n')\n>  \n>      def applyCommit(self, id):\n>          \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n> @@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):\n>          separatorLine = \"######## everything below this line is just the diff #######\\n\"\n>          if not self.prepare_p4_only:\n>              submitTemplate += separatorLine\n> -            submitTemplate += self.get_diff_description(editedFiles)\n> +            submitTemplate += self.get_diff_description(editedFiles, filesToAdd)\n>  \n>          (handle, fileName) = tempfile.mkstemp()\n> -        tmpFile = os.fdopen(handle, \"w+\")\n> +        tmpFile = os.fdopen(handle, \"w+b\")\n>          if self.isWindows:\n>              submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n>          tmpFile.write(submitTemplate)\n> @@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):\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> +                message = message.replace(\"\\r\\n\", \"\\n\")\n> +            submitTemplate = message[:message.index(separatorLine)]\n>              p4_write_pipe(['submit', '-i'], submitTemplate)\n>  \n>              if self.preserveUser:\n> -- \n> 2.0.0\n> \n> \n"},{"id":"243906","messageId":"20140611130658.GA29245@nekage","threadId":"35634","inReplyTo":"20140610223958.GA10049@padd.com","subject":"Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-06-11T13:06:58Z","receivedAt":"2014-06-11T13:06:58Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"On Tue, Jun 10, 2014 at 06:39:58PM -0400, Pete Wyckoff wrote:\n> frrrwww@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:\n> > b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here\n> > is a proper fix, including proper handling for windows end of lines.\n> \n> I guess we don't have test coverage for these cases?  Is this\n> something that should get put into a maintenance release, quickly?\n\nWe have test cases for that, however we need to create a link to git-p4.py\nnamed git-p4 in order for them to work. I did not run the first patch through\nthe tests (see my previous email) because of that. Sorry about that.\n\n> The fix looks good.  It's surprising that none of the tests\n> managed to add a file and trigger the failure.\n> \n> I'll ack this again, as it looks okay, but hope you ran all the\n> unit tests successfully on your machine.\n\nIt works, only one test fail (detect copy), but this test already fails\nwith my two patches reverted.\n\nThis should be applied soon (or alternatively\nb4073bb387ef303c9ac3c044f46d6a8ae6e190f0 should be reverted) in master,\nas in the current state git p4 submit will fail most of the time.\n\nI'll send that with your ack to Junio.\n\nCheers,\n\nMaxime Coste.\n"},{"id":"243907","messageId":"20140611130959.GB29245@nekage","threadId":"35634","inReplyTo":"20140610223958.GA10049@padd.com","subject":"[PATCH] Fix git-p4 submit in non --prepare-p4-only mode","fromName":"Maxime Coste","fromEmail":"frrrwww@gmail.com","sentAt":"2014-06-11T13:09:59Z","receivedAt":"2014-06-11T13:09:59Z","isPatch":true,"sender":{"key":"frrrwww@gmail.com","avatar":"https://avatars.githubusercontent.com/u/51221?v=4"},"body":"b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here\nis a proper fix, including proper handling for windows end of lines.\n\nSigned-off-by: Maxime Coste <frrrwww@gmail.com>\nAcked-by: Pete Wyckoff <pw@padd.com> \n---\n git-p4.py | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 7bb0f73..ff132b2 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):\n             if response == 'n':\n                 return False\n \n-    def get_diff_description(self, editedFiles):\n+    def get_diff_description(self, editedFiles, filesToAdd):\n         # diff\n         if os.environ.has_key(\"P4DIFF\"):\n             del(os.environ[\"P4DIFF\"])\n@@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):\n                 newdiff += \"+\" + line\n             f.close()\n \n-        return diff + newdiff\n+        return (diff + newdiff).replace('\\r\\n', '\\n')\n \n     def applyCommit(self, id):\n         \"\"\"Apply one commit, return True if it succeeded.\"\"\"\n@@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):\n         separatorLine = \"######## everything below this line is just the diff #######\\n\"\n         if not self.prepare_p4_only:\n             submitTemplate += separatorLine\n-            submitTemplate += self.get_diff_description(editedFiles)\n+            submitTemplate += self.get_diff_description(editedFiles, filesToAdd)\n \n         (handle, fileName) = tempfile.mkstemp()\n-        tmpFile = os.fdopen(handle, \"w+\")\n+        tmpFile = os.fdopen(handle, \"w+b\")\n         if self.isWindows:\n             submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n         tmpFile.write(submitTemplate)\n@@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):\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+                message = message.replace(\"\\r\\n\", \"\\n\")\n+            submitTemplate = message[:message.index(separatorLine)]\n             p4_write_pipe(['submit', '-i'], submitTemplate)\n \n             if self.preserveUser:\n-- \n2.0.0\n"},{"id":"243908","messageId":"20140611133602.GA17043@padd.com","threadId":"35634","inReplyTo":"20140611130658.GA29245@nekage","subject":"Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2014-06-11T13:36:02Z","receivedAt":"2014-06-11T13:36:02Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"frrrwww@gmail.com wrote on Wed, 11 Jun 2014 14:06 +0100:\n> On Tue, Jun 10, 2014 at 06:39:58PM -0400, Pete Wyckoff wrote:\n> > frrrwww@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:\n> > > b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here\n> > > is a proper fix, including proper handling for windows end of lines.\n> > \n> > I guess we don't have test coverage for these cases?  Is this\n> > something that should get put into a maintenance release, quickly?\n> \n> We have test cases for that, however we need to create a link to git-p4.py\n> named git-p4 in order for them to work. I did not run the first patch through\n> the tests (see my previous email) because of that. Sorry about that.\n\nThe secret is to \"build\" the code before running tests, just like\nwhen working on .c files.  I tend to do something like:\n\n    make git-p4 && (cd t ; make T=\"$(echo t98*)\") ; pkill p4d\n\nThanks for catching the problem quickly and fixing it.\n\n\t\t-- Pete\n"}]}