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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 01/26/2013 10:44 PM, Junio C Hamano wrote:
> John Keeping <john@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 < 0x02000000:
>> +    # 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 < 0x03000000:
        pathbytes = repo.path
    elif sys.hexversion < 0x03010000:
        # 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()

Michael

[1] http://www.python.org/dev/peps/pep-0383/

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/