Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
On Sat, Jan 26, 2013 at 09:30:00PM -0800, Junio C Hamano wrote:
> Michael Haggerty 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
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 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 "", line 4, in
> 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
On 01/27/2013 06:30 AM, Sverre Rabbelier wrote: > On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty > 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 b"value". (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
On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty 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
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
Michael Haggerty 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
On 01/26/2013 10:44 PM, Junio C Hamano wrote:
> John Keeping 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 "", line 4, in
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 "", line 4, in
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.e
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
John Keeping 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
Under Python 3 'hasher.update(...)' must take a byte string and not a
unicode string. Explicitly encode the argument to this method as UTF-8
bytes. This is safe since we are encoding a Python Unicode string to a
Unicode 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
---
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.
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(
--
1.8.1.1
--
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
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
---
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
