git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode

From
PWPete Wyckoff <pw@padd.com>
Date
Jun 10, 2014, 22:39 UTC
Message-ID
<20140610223958.GA10049@padd.com>
In-Reply-To
<20140610121446.GA25634@nekage>
frrrwww@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
Show 56 quoted lines
> Signed-off-by: Maxime Coste <frrrwww@gmail.com>
> ---
>  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
> 
> 
Previous: Maxime CosteNext: Maxime Coste
Message 10 of 13 in “git-p4: Do not include diff in spec file when just preparing p4”
  1. git-p4: Do not include diff in spec file when just preparing p4Maxime Coste, Jan 10, 2014
  2. Pete WyckoffJan 12, 2014
  3. Maxime CosteJan 13, 2014
  4. Pete WyckoffJan 14, 2014
  5. git-p4: Do not include diff in spec file when just preparing p4Maxime Coste, May 24, 2014
  6. Maxime CosteMay 24, 2014
  7. Pete WyckoffMay 24, 2014
  8. git-p4: Do not include diff in spec file when just preparing p4Maxime Coste, May 24, 2014
  9. Fix git-p4 submit in non --prepare-p4-only modeMaxime Coste, Jun 10, 2014
  10. Pete WyckoffJun 10, 2014
  11. Maxime CosteJun 11, 2014
  12. Pete WyckoffJun 11, 2014
  13. Fix git-p4 submit in non --prepare-p4-only modeMaxime Coste, Jun 11, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.