Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode
frrr...@gmail.com wrote on Wed, 11 Jun 2014 14:06 +0100: > On Tue, Jun 10, 2014 at 06:39:58PM -0400, Pete Wyckoff wrote: > > frrr...@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100: > > > b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here > > > is a proper fix, including proper handling for windows end of lines. > > > > I guess we don't have test coverage for these cases? Is this > > something that should get put into a maintenance release, quickly? > > We have test cases for that, however we need to create a link to git-p4.py > named git-p4 in order for them to work. I did not run the first patch through > the tests (see my previous email) because of that. Sorry about that. The secret is to "build" the code before running tests, just like when working on .c files. I tend to do something like: make git-p4 && (cd t ; make T="$(echo t98*)") ; pkill p4d Thanks for catching the problem quickly and fixing it. -- Pete -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix git-p4 submit in non --prepare-p4-only mode
b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here
is a proper fix, including proper handling for windows end of lines.
Signed-off-by: Maxime Coste
Acked-by: Pete Wyckoff
---
git-p4.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7bb0f73..ff132b2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):
if response == 'n':
return False
-def get_diff_description(self, editedFiles):
+def get_diff_description(self, editedFiles, filesToAdd):
# diff
if os.environ.has_key("P4DIFF"):
del(os.environ["P4DIFF"])
@@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):
newdiff += "+" + line
f.close()
-return diff + newdiff
+return (diff + newdiff).replace('\r\n', '\n')
def applyCommit(self, id):
"""Apply one commit, return True if it succeeded."""
@@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):
separatorLine = " everything below this line is just the diff
###\n"
if not self.prepare_p4_only:
submitTemplate += separatorLine
-submitTemplate += self.get_diff_description(editedFiles)
+submitTemplate += self.get_diff_description(editedFiles,
filesToAdd)
(handle, fileName) = tempfile.mkstemp()
-tmpFile = os.fdopen(handle, "w+")
+tmpFile = os.fdopen(handle, "w+b")
if self.isWindows:
submitTemplate = submitTemplate.replace("\n", "\r\n")
tmpFile.write(submitTemplate)
@@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):
tmpFile = open(fileName, "rb")
message = tmpFile.read()
tmpFile.close()
-submitTemplate = message[:message.index(separatorLine)]
if self.isWindows:
-submitTemplate = submitTemplate.replace("\r\n", "\n")
+message = message.replace("\r\n", "\n")
+submitTemplate = message[:message.index(separatorLine)]
p4_write_pipe(['submit', '-i'], submitTemplate)
if self.preserveUser:
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode
On Tue, Jun 10, 2014 at 06:39:58PM -0400, Pete Wyckoff wrote: > frrr...@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100: > > b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here > > is a proper fix, including proper handling for windows end of lines. > > I guess we don't have test coverage for these cases? Is this > something that should get put into a maintenance release, quickly? We have test cases for that, however we need to create a link to git-p4.py named git-p4 in order for them to work. I did not run the first patch through the tests (see my previous email) because of that. Sorry about that. > The fix looks good. It's surprising that none of the tests > managed to add a file and trigger the failure. > > I'll ack this again, as it looks okay, but hope you ran all the > unit tests successfully on your machine. It works, only one test fail (detect copy), but this test already fails with my two patches reverted. This should be applied soon (or alternatively b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 should be reverted) in master, as in the current state git p4 submit will fail most of the time. I'll send that with your ack to Junio. Cheers, Maxime Coste. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode
frrr...@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:
> b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here
> is a proper fix, including proper handling for windows end of lines.
I guess we don't have test coverage for these cases? Is this
something that should get put into a maintenance release, quickly?
The fix looks good. It's surprising that none of the tests
managed to add a file and trigger the failure.
I'll ack this again, as it looks okay, but hope you ran all the
unit tests successfully on your machine.
-- Pete
> Signed-off-by: Maxime Coste
> ---
> git-p4.py | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 7bb0f73..ff132b2 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):
> if response == 'n':
> return False
>
> -def get_diff_description(self, editedFiles):
> +def get_diff_description(self, editedFiles, filesToAdd):
> # diff
> if os.environ.has_key("P4DIFF"):
> del(os.environ["P4DIFF"])
> @@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):
> newdiff += "+" + line
> f.close()
>
> -return diff + newdiff
> +return (diff + newdiff).replace('\r\n', '\n')
>
> def applyCommit(self, id):
> """Apply one commit, return True if it succeeded."""
> @@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):
> separatorLine = " everything below this line is just the
> diff ###\n"
> if not self.prepare_p4_only:
> submitTemplate += separatorLine
> -submitTemplate += self.get_diff_description(editedFiles)
> +submitTemplate += self.get_diff_description(editedFiles,
> filesToAdd)
>
> (handle, fileName) = tempfile.mkstemp()
> -tmpFile = os.fdopen(handle, "w+")
> +tmpFile = os.fdopen(handle, "w+b")
> if self.isWindows:
> submitTemplate = submitTemplate.replace("\n", "\r\n")
> tmpFile.write(submitTemplate)
> @@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):
> tmpFile = open(fileName, "rb")
> message = tmpFile.read()
> tmpFile.close()
> -submitTemplate = message[:message.index(separatorLine)]
> if self.isWindows:
> -submitTemplate = submitTemplate.replace("\r\n", "\n")
> +message = message.replace("\r\n", "\n")
> +submitTemplate = message[:message.index(separatorLine)]
> p4_write_pipe(['submit', '-i'], submitTemplate)
>
> if self.preserveUser:
> --
> 2.0.0
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix git-p4 submit in non --prepare-p4-only mode
b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here
is a proper fix, including proper handling for windows end of lines.
Signed-off-by: Maxime Coste
---
git-p4.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7bb0f73..ff132b2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):
if response == 'n':
return False
-def get_diff_description(self, editedFiles):
+def get_diff_description(self, editedFiles, filesToAdd):
# diff
if os.environ.has_key("P4DIFF"):
del(os.environ["P4DIFF"])
@@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):
newdiff += "+" + line
f.close()
-return diff + newdiff
+return (diff + newdiff).replace('\r\n', '\n')
def applyCommit(self, id):
"""Apply one commit, return True if it succeeded."""
@@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):
separatorLine = " everything below this line is just the diff
###\n"
if not self.prepare_p4_only:
submitTemplate += separatorLine
-submitTemplate += self.get_diff_description(editedFiles)
+submitTemplate += self.get_diff_description(editedFiles,
filesToAdd)
(handle, fileName) = tempfile.mkstemp()
-tmpFile = os.fdopen(handle, "w+")
+tmpFile = os.fdopen(handle, "w+b")
if self.isWindows:
submitTemplate = submitTemplate.replace("\n", "\r\n")
tmpFile.write(submitTemplate)
@@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):
tmpFile = open(fileName, "rb")
message = tmpFile.read()
tmpFile.close()
-submitTemplate = message[:message.index(separatorLine)]
if self.isWindows:
-submitTemplate = submitTemplate.replace("\r\n", "\n")
+message = message.replace("\r\n", "\n")
+submitTemplate = message[:message.index(separatorLine)]
p4_write_pipe(['submit', '-i'], submitTemplate)
if self.preserveUser:
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
