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

From: John Keeping <john@keeping.me.uk>
Date: Jan 27, 2013, 14:13 UTC
Message-ID: <20130127141329.GN7498@serenity.lan>
In-Reply-To: <5104B0B5.1030501@alum.mit.edu>

On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:

Show 14 quoted lines

> On 01/26/2013 10:44 PM, Junio C Hamano wrote:
> > John Keeping <john@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.

Show 5 quoted lines

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

Show 25 quoted lines

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

Show 17 quoted lines

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

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