techtonik / python-patch

Library to parse and apply unified diffs
https://pypi.python.org/pypi/patch
106 stars 63 forks source link

xnormapth is not cross platform @ r196 #24

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi Anatoly:

the code in 
http://code.google.com/p/python-patch/source/browse/trunk/patch.py#89 is not 
cross platform and will yield different results when running on posix or 
windows.

What steps will reproduce the problem?
Using a patch like this: http://drupal.org/files/issues/drupal_upload.patch

diff -Nup C:\Documents and Settings\Boki\Desktop\head\patches\drupal6/drupal.js 
C:\Documents and Settings\Boki\Desktop\head\patches\parallel uploads/drupal.js
--- C:\Documents and 
Settings\Boki\Desktop\head\patches\drupal6/drupal.js    2007-06-08 
14:52:00.000000000 +0200
+++ C:\Documents and Settings\Boki\Desktop\head\patches\parallel 
uploads/drupal.js   2007-06-16 19:27:20.953125000 +0200

We will get on posix after parsing the patch:

source: Documents and Settings\Boki\Desktop\head\patches\drupal6/drupal.js
target: Documents and Settings\Boki\Desktop\head\patches\parallel 
uploads/drupal.js

And on Windows:

source: Documents and Settings/Boki/Desktop/head/patches/drupal6/drupal.js
target: Documents and Settings/Boki/Desktop/head/patches/parallel 
uploads/drupal.js

What is the expected output? What do you see instead?
=>The output should be the same on all OS

Actual output on win:

Python 2.6.6 (r266:84297, Aug 24 2010, 18:46:32) [MSC v.1500 32 bit (Intel)] on 
win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import patch
>>> p=patch.fromurl('http://drupal.org/files/issues/drupal_upload.patch')
>>> p.items[0].source
'Documents and Settings/Boki/Desktop/head/patches/drupal6/drupal.js'
>>> p.items[0].target
'Documents and Settings/Boki/Desktop/head/patches/parallel uploads/drupal.js'

Actual output on posix (here with cugwin, but the same behavior has been 
verified on Linux and Macosx):
Python 2.6.5 (r265:79063, Jun 12 2010, 17:07:01)
[GCC 4.3.4 20090804 (release) 1] on cygwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import patch
>>> p=patch.fromurl('http://drupal.org/files/issues/drupal_upload.patch')
>>> p.items[0].source
'Documents and Settings\\Boki\\Desktop\\head\\patches\\drupal6/drupal.js'
>>> p.items[0].target
'Documents and Settings\\Boki\\Desktop\\head\\patches\\parallel 
uploads/drupal.js'

Tell me if you want a patch... The strategy I use in general for cross-os path 
handling is to convert at the boundaries any path to posix, and use internally 
the posixpath module for all path handling, converting back to os.path-specific 
path on demand for thing that require actual IOs to os files.

--
Cordially
Philippe

Original issue reported on code.google.com by pombreda...@gmail.com on 20 Apr 2013 at 11:01

GoogleCodeExporter commented 9 years ago
Sorry for a looong delay and thanks for the hint.
I was not aware of the issue until I saw this:

https://drone.io/techtonik/python-patch/1

The reason is that I never had to run tests on Linux.
At first I thought about implementing the whole
escaping logic from scratch, but building from 
predictable POSIX way makes code more readable.

Original comment by techtonik@gmail.com on 3 Mar 2014 at 7:48

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r245.

Original comment by techtonik@gmail.com on 3 Mar 2014 at 7:57

GoogleCodeExporter commented 9 years ago
Thanks mucho sir!

Original comment by pombreda...@gmail.com on 3 Mar 2014 at 8:10

GoogleCodeExporter commented 9 years ago
No problem. =)

There are still two more things to be fixed on Linux.
https://drone.io/techtonik/python-patch/3
But these are probably caused by the recent changes in test suite.

Original comment by techtonik@gmail.com on 3 Mar 2014 at 8:16