Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-27 Thread Michael Haggerty
On 01/27/2013 06:30 AM, Sverre Rabbelier wrote:
 On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:
 So to handle all of the cases across Python versions as closely as
 possible to the old 2.x code, it might be necessary to make the code
 explicitly depend on the Python version number, like:
 
 Does this all go away if we restrict ourselves to python 2.6 and just
 use the b prefix?

repo.path ultimately comes from the command line, which means that it is
a bytestring under Python 2.x and a Unicode string under Python 3.x.  It
does not come from a literal that could be changed to bvalue.  (Nor is
a six.b()-like function helpful, if that is what you meant; that is also
intended to wrap literal strings.)

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-27 Thread John Keeping
On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:
 On 01/26/2013 10:44 PM, Junio C Hamano wrote:
  John Keeping j...@keeping.me.uk writes:
  @@ -45,7 +45,7 @@ def get_repo(alias, url):
   repo.get_head()
   
   hasher = _digest()
  -hasher.update(repo.path)
  +hasher.update(repo.path.encode('utf-8'))
   repo.hash = hasher.hexdigest()
   
   repo.get_base_path = lambda base: os.path.join(
 
 This will still fail under Python 2.x if repo.path is a byte string that
 contains non-ASCII characters.

I had forgotten about Python 2 while doing this.

 And it will fail under Python 3.1 and
 later if repo.path contains characters using the surrogateescape
 encoding option [1], as it will if the original command-line argument
 contained bytes that cannot be decoded into Unicode using the user's
 default encoding:

Interesting.  I wasn't aware of the surrogateescape error handler.

 'surrogateescape' is not supported in Python 3.0, but I think it would
 be quite acceptable only to support Python 3.x for x = 1.

I agree.

 But 'surrogateescape' doesn't seem to be supported at all in Python 2.x
 (I tested 2.7.3 and it's not there).
 
 Here you don't really need byte-for-byte correctness; it would be enough
 to get *some* byte string that is unique for a given input (ideally,
 consistent with ASCII or UTF-8 for backwards compatibility).  So you
 could use
 
 b = s.encode('utf-8', 'backslashreplace')
 
 Unfortunately, this doesn't work under Python 2.x:
 
 $ python2 -c 
 import sys
 print(repr(sys.argv[1]))
 print(repr(sys.argv[1].encode('utf-8', 'backslashreplace')))
  $(echo français|iconv -t latin1)
 'fran\xe7ais'
 Traceback (most recent call last):
   File string, line 4, in module
 UnicodeDecodeError: 'ascii' codec can't decode byte 0xe7 in position
 4: ordinal not in range(128)
 
 Apparently when you call bytestring.encode(), Python first tries to
 decode the string to Unicode using the 'ascii' encoding.

Actually it appears to use sys.getdefaultencoding() to do this initial
decode.  Not that it makes much difference here since the failure is the
same.

 So to handle all of the cases across Python versions as closely as
 possible to the old 2.x code, it might be necessary to make the code
 explicitly depend on the Python version number, like:
 
 hasher = _digest()
 if sys.hexversion  0x0300:
 pathbytes = repo.path
 elif sys.hexversion  0x0301:
 # If support for Python 3.0.x is desired (note: result can
 # be different in this case than under 2.x or 3.1+):
 pathbytes = repo.path.encode(sys.getfilesystemencoding(),
 'backslashreplace')
 else
 pathbytes = repo.path.encode(sys.getfilesystemencoding(),
 'surrogateescape')
 hasher.update(pathbytes)
 repo.hash = hasher.hexdigest()

If we don't want to put a version check in it probably wants to look
like this (ignoring Python 3.0 since I don't think we need to support
it):

hasher = _digest()
try:
codecs.lookup_error('surrogateescape')
except LookupError:
pathbytes = repo.path
else:
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
 'surrogateescape')
hasher.update(pathbytes)
repo.hash = hasher.hexdigest()

The version with a version check seems better to me, although this
should probably be a utility function.


John
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-27 Thread John Keeping
On Sat, Jan 26, 2013 at 09:30:00PM -0800, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
  This will still fail under Python 2.x if repo.path is a byte string that
  contains non-ASCII characters.  And it will fail under Python 3.1 and
  later if repo.path contains characters using the surrogateescape
  encoding option [1],...
  Here you don't really need byte-for-byte correctness; it would be enough
  to get *some* byte string that is unique for a given input ...
 
 Yeek.
 
 As we do not care about the actual value at all, how about doing
 something like this instead?
 
 +hasher.update(..join([str(ord(c)) for c in repo.path]))

This doesn't solve the original problem since we're still ending up with
a Unicode string.  If we wanted something like this it would need to be:

hasher.update(b'.'.join([b'%X' % ord(c) for c in repo.path]))

which limits us to Python 2.6 and later and seems to me to be less clear
than introducing an encode_filepath helper function using Michael's
suggestion.


John
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-26 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 Junio, can you replace the queued 0846b0c (git-remote-testpy: hash bytes
 explicitly) with this?

 I hadn't realised that the hex encoding we chose before is a bytes to
 bytes encoding so it just fails with an error on Python 3 in the same
 way as the original code.

 Since we want to convert a Unicode string to bytes I think UTF-8 really
 is the best option here.

Ahh.  I think it is already in next, so this needs to be turned
into an incremental to flip 'hex' to 'utf-8', with the justification
being these five lines above.

Thanks for catching.


  git-remote-testpy.py | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/git-remote-testpy.py b/git-remote-testpy.py
 index d94a66a..f8dc196 100644
 --- a/git-remote-testpy.py
 +++ b/git-remote-testpy.py
 @@ -31,9 +31,9 @@ from git_remote_helpers.git.exporter import GitExporter
  from git_remote_helpers.git.importer import GitImporter
  from git_remote_helpers.git.non_local import NonLocalGit
  
 -if sys.hexversion  0x01050200:
 -# os.makedirs() is the limiter
 -sys.stderr.write(git-remote-testgit: requires Python 1.5.2 or later.\n)
 +if sys.hexversion  0x0200:
 +# string.encode() is the limiter
 +sys.stderr.write(git-remote-testgit: requires Python 2.0 or later.\n)
  sys.exit(1)
  
  def get_repo(alias, url):
 @@ -45,7 +45,7 @@ def get_repo(alias, url):
  repo.get_head()
  
  hasher = _digest()
 -hasher.update(repo.path)
 +hasher.update(repo.path.encode('utf-8'))
  repo.hash = hasher.hexdigest()
  
  repo.get_base_path = lambda base: os.path.join(
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-26 Thread Michael Haggerty
On 01/26/2013 10:44 PM, Junio C Hamano wrote:
 John Keeping j...@keeping.me.uk writes:
 
 Junio, can you replace the queued 0846b0c (git-remote-testpy: hash bytes
 explicitly) with this?

 I hadn't realised that the hex encoding we chose before is a bytes to
 bytes encoding so it just fails with an error on Python 3 in the same
 way as the original code.

 Since we want to convert a Unicode string to bytes I think UTF-8 really
 is the best option here.
 
 Ahh.  I think it is already in next, so this needs to be turned
 into an incremental to flip 'hex' to 'utf-8', with the justification
 being these five lines above.
 
 Thanks for catching.
 

  git-remote-testpy.py | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/git-remote-testpy.py b/git-remote-testpy.py
 index d94a66a..f8dc196 100644
 --- a/git-remote-testpy.py
 +++ b/git-remote-testpy.py
 @@ -31,9 +31,9 @@ from git_remote_helpers.git.exporter import GitExporter
  from git_remote_helpers.git.importer import GitImporter
  from git_remote_helpers.git.non_local import NonLocalGit
  
 -if sys.hexversion  0x01050200:
 -# os.makedirs() is the limiter
 -sys.stderr.write(git-remote-testgit: requires Python 1.5.2 or 
 later.\n)
 +if sys.hexversion  0x0200:
 +# string.encode() is the limiter
 +sys.stderr.write(git-remote-testgit: requires Python 2.0 or later.\n)
  sys.exit(1)
  
  def get_repo(alias, url):
 @@ -45,7 +45,7 @@ def get_repo(alias, url):
  repo.get_head()
  
  hasher = _digest()
 -hasher.update(repo.path)
 +hasher.update(repo.path.encode('utf-8'))
  repo.hash = hasher.hexdigest()
  
  repo.get_base_path = lambda base: os.path.join(

This will still fail under Python 2.x if repo.path is a byte string that
contains non-ASCII characters.  And it will fail under Python 3.1 and
later if repo.path contains characters using the surrogateescape
encoding option [1], as it will if the original command-line argument
contained bytes that cannot be decoded into Unicode using the user's
default encoding:

$ python3 --version
Python 3.2.3
$ python3 -c 
import sys
print(repr(sys.argv[1]))
print(repr(sys.argv[1].encode('utf-8')))
 $(echo français|iconv -t latin1)
'fran\udce7ais'
Traceback (most recent call last):
  File string, line 4, in module
UnicodeEncodeError: 'utf-8' codec can't encode character '\udce7' in
position 4: surrogates not allowed

I'm not sure what happens in Python 3.0.

I think the modern way to handle this situation in Python 3.1+ is via
PEP 383's surrogateescape encoding option [1]:

repo.path.encode('utf-8', 'surrogateescape')

Basically, byte strings that come from the OS are automatically decoded
into Unicode strings using

s = b.decode(sys.getfilesystemencoding(), 'surrogateescape')

If the string needs to be passed back to the filesystem as a byte string
it is via

b = s.encode(sys.getfilesystemencoding(), 'surrogateescape')

My understanding is that the surrogateescape mechanism guarantees that
the round-trip bytestring - string - bytestring gives back the
original byte string, which is what you want for things like filenames.
 But a Unicode string that contains surrogate escape characters *cannot*
be encoded without the 'surrogateescape' option.

'surrogateescape' is not supported in Python 3.0, but I think it would
be quite acceptable only to support Python 3.x for x = 1.

But 'surrogateescape' doesn't seem to be supported at all in Python 2.x
(I tested 2.7.3 and it's not there).

Here you don't really need byte-for-byte correctness; it would be enough
to get *some* byte string that is unique for a given input (ideally,
consistent with ASCII or UTF-8 for backwards compatibility).  So you
could use

b = s.encode('utf-8', 'backslashreplace')

Unfortunately, this doesn't work under Python 2.x:

$ python2 -c 
import sys
print(repr(sys.argv[1]))
print(repr(sys.argv[1].encode('utf-8', 'backslashreplace')))
 $(echo français|iconv -t latin1)
'fran\xe7ais'
Traceback (most recent call last):
  File string, line 4, in module
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe7 in position
4: ordinal not in range(128)

Apparently when you call bytestring.encode(), Python first tries to
decode the string to Unicode using the 'ascii' encoding.

So to handle all of the cases across Python versions as closely as
possible to the old 2.x code, it might be necessary to make the code
explicitly depend on the Python version number, like:

hasher = _digest()
if sys.hexversion  0x0300:
pathbytes = repo.path
elif sys.hexversion  0x0301:
# If support for Python 3.0.x is desired (note: result can
# be different in this case than under 2.x or 3.1+):
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
'backslashreplace')
else
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
'surrogateescape')

Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-26 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 This will still fail under Python 2.x if repo.path is a byte string that
 contains non-ASCII characters.  And it will fail under Python 3.1 and
 later if repo.path contains characters using the surrogateescape
 encoding option [1],...
 Here you don't really need byte-for-byte correctness; it would be enough
 to get *some* byte string that is unique for a given input ...

Yeek.

As we do not care about the actual value at all, how about doing
something like this instead?

 git-remote-testgit.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..705750d 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -40,7 +40,7 @@ def get_repo(alias, url):
 repo.get_head()
 
 hasher = _digest()
-hasher.update(repo.path)
+hasher.update(..join([str(ord(c)) for c in repo.path]))
 repo.hash = hasher.hexdigest()
 
 repo.get_base_path = lambda base: os.path.join(
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-26 Thread Sverre Rabbelier
On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 So to handle all of the cases across Python versions as closely as
 possible to the old 2.x code, it might be necessary to make the code
 explicitly depend on the Python version number, like:

Does this all go away if we restrict ourselves to python 2.6 and just
use the b prefix?

--
Cheers,

Sverre Rabbelier
--
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 v3 6/8] git-remote-testpy: hash bytes explicitly

2013-01-20 Thread John Keeping
Under Python 3 'hasher.update(...)' must take a byte string and not a
unicode string.  Explicitly encode the argument to this method to hex
bytes so that we don't need to worry about failures to encode that might
occur if we chose a textual encoding.

This changes the directory used by git-remote-testpy for its git mirror
of the remote repository, but this tool should not have any serious
users as it is used primarily to test the Python remote helper
framework.

The use of encode() moves the required Python version forward to 2.0.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 git-remote-testpy.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index d94a66a..197b7be 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -31,9 +31,9 @@ from git_remote_helpers.git.exporter import GitExporter
 from git_remote_helpers.git.importer import GitImporter
 from git_remote_helpers.git.non_local import NonLocalGit
 
-if sys.hexversion  0x01050200:
-# os.makedirs() is the limiter
-sys.stderr.write(git-remote-testgit: requires Python 1.5.2 or later.\n)
+if sys.hexversion  0x0200:
+# string.encode() is the limiter
+sys.stderr.write(git-remote-testgit: requires Python 2.0 or later.\n)
 sys.exit(1)
 
 def get_repo(alias, url):
@@ -45,7 +45,7 @@ def get_repo(alias, url):
 repo.get_head()
 
 hasher = _digest()
-hasher.update(repo.path)
+hasher.update(repo.path.encode('hex'))
 repo.hash = hasher.hexdigest()
 
 repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1.353.gc992d5a.dirty

--
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