threads / patch / 32676

v3, 8 partsPython 3 support for git_remote_helpers

Subject: [PATCH v3 0/8] Python 3 support for git_remote_helpers

## tl;dr

38 messages between Jan 20, 2013 and Feb 5, 2013. Diffs are folded; open one to read it.

replies: 37people: 6as markdown or json

John Keeping· Jan 20, 2013, 13:15 UTC · lore
This series does enough so that everything except git-p4 runs under
Python 3.
                                                                                 
As discussed with Pete, it may not make sense to change git-p4 to
support Python 3 until Perforce's Python output mode is changed.  So
does it make sense to merge this now and say "use Python 2 if you want
git-p4"?
                                                                                 
Changes since v2:
 - Change reference URL in commit message of patch 4
   (git_remote_helpers: Use 2to3 if building with Python 3) to point at
   Python documentation instead of the Python wiki.
 - Add a comment in patch 7 (git-remote-testpy: don't do unbuffered text
   I/O), as suggested by Sverre.
John Keeping (8):
  git_remote_helpers: Allow building with Python 3
  git_remote_helpers: fix input when running under Python 3
  git_remote_helpers: Force rebuild if python version changes
  git_remote_helpers: Use 2to3 if building with Python 3
  svn-fe: allow svnrdump_sim.py to run with Python 3
  git-remote-testpy: hash bytes explicitly
  git-remote-testpy: don't do unbuffered text I/O
  git-remote-testpy: call print as a function
 contrib/svn-fe/svnrdump_sim.py     |  4 ++--
 git-remote-testpy.py               | 46 +++++++++++++++++++++-----------------
 git_remote_helpers/.gitignore      |  1 +
 git_remote_helpers/Makefile        | 10 +++++++--
 git_remote_helpers/git/importer.py |  9 +++++---
 git_remote_helpers/setup.py        | 10 +++++++++
 6 files changed, 52 insertions(+), 28 deletions(-)
-- 
1.8.1.353.gc992d5a.dirty
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 1/8] git_remote_helpers: Allow building with Python 3

Change inline Python to call "print" as a function not a statement.

This is harmless because Python 2 will see the parentheses as redundant grouping but they are necessary to run this code with Python 3.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to git_remote_helpers/Makefile +1 −1
diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
index 74b05dc..f65f064 100644
--- a/git_remote_helpers/Makefile
+++ b/git_remote_helpers/Makefile
@@ -23,7 +23,7 @@ endif
 
 PYLIBDIR=$(shell $(PYTHON_PATH) -c \
 	 "import sys; \
-	 print 'lib/python%i.%i/site-packages' % sys.version_info[:2]")
+	 print('lib/python%i.%i/site-packages' % sys.version_info[:2])")
 
 all: $(pysetupfile)
 	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
-- 
1.8.1.353.gc992d5a.dirty
Sverre Rabbelier· Jan 23, 2013, 18:49 UTC · re: John Keeping · lore

Re: [PATCH v3 1/8] git_remote_helpers: Allow building with Python 3

On Sun, Jan 20, 2013 at 5:15 AM, John Keeping <john@keeping.me.uk> wrote:
Show 6 quoted lines
> Change inline Python to call "print" as a function not a statement.
>
> This is harmless because Python 2 will see the parentheses as redundant
> grouping but they are necessary to run this code with Python 3.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>

-- Cheers,

Sverre Rabbelier
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

Although 2to3 will fix most issues in Python 2 code to make it run under Python 3, it does not handle the new strict separation between byte strings and unicode strings. There is one instance in git_remote_helpers where we are caught by this, which is when reading refs from "git for-each-ref".

Fix this by operating on the returned string as a byte string rather than a unicode string. As this method is currently only used internally by the class this does not affect code anywhere else.

Note that we cannot use byte strings in the source as the 'b' prefix is not supported before Python 2.7 so in order to maintain compatibility with the maximum range of Python versions we use an explicit call to encode().

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/git/importer.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to git_remote_helpers/git/importer.py +6 −3
diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index e28cc8f..d3f90e1 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -18,13 +18,16 @@ class GitImporter(object):
 
     def get_refs(self, gitdir):
         """Returns a dictionary with refs.
+
+        Note that the keys in the returned dictionary are byte strings as
+        read from git.
         """
         args = ["git", "--git-dir=" + gitdir, "for-each-ref", "refs/heads"]
-        lines = check_output(args).strip().split('\n')
+        lines = check_output(args).strip().split('\n'.encode('ascii'))
         refs = {}
         for line in lines:
-            value, name = line.split(' ')
-            name = name.strip('commit\t')
+            value, name = line.split(' '.encode('ascii'))
+            name = name.strip('commit\t'.encode('ascii'))
             refs[name] = value
         return refs
 
-- 
1.8.1.353.gc992d5a.dirty
Sverre Rabbelier· Jan 23, 2013, 19:20 UTC · re: John Keeping · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

On Sun, Jan 20, 2013 at 5:15 AM, John Keeping <john@keeping.me.uk> wrote:
Show 14 quoted lines
> Although 2to3 will fix most issues in Python 2 code to make it run under
> Python 3, it does not handle the new strict separation between byte
> strings and unicode strings.  There is one instance in
> git_remote_helpers where we are caught by this, which is when reading
> refs from "git for-each-ref".
>
> Fix this by operating on the returned string as a byte string rather
> than a unicode string.  As this method is currently only used internally
> by the class this does not affect code anywhere else.
>
> Note that we cannot use byte strings in the source as the 'b' prefix is
> not supported before Python 2.7 so in order to maintain compatibility
> with the maximum range of Python versions we use an explicit call to
> encode().

The three patches that deal with .encode() stuff (2, 7, 8) make me a bit uncomfortable, as they add some significant complexity to our python code. Is this the recommended way to deal with this (similar to the other patch where you linked to the python wiki explaining)?

As one datapoint, it seems that it's actually Python 2.6 that introduces the b prefix.

http://www.python.org/dev/peps/pep-3112/
When did we last revisit what minimal python version we are ok with requiring?

-- Cheers,

Sverre Rabbelier
John Keeping· Jan 23, 2013, 19:47 UTC · re: Sverre Rabbelier · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

On Wed, Jan 23, 2013 at 11:20:39AM -0800, Sverre Rabbelier wrote:
Show 20 quoted lines
> On Sun, Jan 20, 2013 at 5:15 AM, John Keeping <john@keeping.me.uk> wrote:
> > Although 2to3 will fix most issues in Python 2 code to make it run under
> > Python 3, it does not handle the new strict separation between byte
> > strings and unicode strings.  There is one instance in
> > git_remote_helpers where we are caught by this, which is when reading
> > refs from "git for-each-ref".
> >
> > Fix this by operating on the returned string as a byte string rather
> > than a unicode string.  As this method is currently only used internally
> > by the class this does not affect code anywhere else.
> >
> > Note that we cannot use byte strings in the source as the 'b' prefix is
> > not supported before Python 2.7 so in order to maintain compatibility
> > with the maximum range of Python versions we use an explicit call to
> > encode().
> 
> The three patches that deal with .encode() stuff (2, 7, 8) make me a
> bit uncomfortable, as they add some significant complexity to our
> python code. Is this the recommended way to deal with this (similar to
> the other patch where you linked to the python wiki explaining)?
The best I can offer is this:
http://docs.python.org/3/howto/pyporting.html#deal-with-the-bytes-string-dichotomy

Their recommendation is to use the b() function from the six project, but given that we don't need it in too many places I prefer the approach I took here to adding a thirdparty dependency.

Show 6 quoted lines
> As one datapoint, it seems that it's actually Python 2.6 that
> introduces the b prefix.
> 
> http://www.python.org/dev/peps/pep-3112/
> 
> When did we last revisit what minimal python version we are ok with requiring?

I was wondering if people would weigh in discussing that in response to [1] but no one has commented on that part of it. As another datapoint, Brandon Casey was suggesting patching git-p4.py to support Python 2.4 [2].

[1] http://article.gmane.org/gmane.comp.version-control.git/213920 [2] http://article.gmane.org/gmane.comp.version-control.git/214048

John
Sverre Rabbelier· Jan 23, 2013, 20:14 UTC · re: John Keeping · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

On Wed, Jan 23, 2013 at 11:47 AM, John Keeping <john@keeping.me.uk> wrote:
Show 9 quoted lines
>> When did we last revisit what minimal python version we are ok with requiring?
>
> I was wondering if people would weigh in discussing that in response to
> [1] but no one has commented on that part of it.  As another datapoint,
> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
> [2].
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
> [2] http://article.gmane.org/gmane.comp.version-control.git/214048

I for one would be happy to kill off support for anything older than 2.6 (which had it's latest release on October 1st, 2008).

Junio, how have we decided in the past which version of x to support?

-- Cheers,

Sverre Rabbelier
Junio C Hamano· Jan 23, 2013, 20:36 UTC · re: Sverre Rabbelier · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

Sverre Rabbelier <srabbelier@gmail.com> writes:
Show 15 quoted lines
> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping <john@keeping.me.uk> wrote:
>>> When did we last revisit what minimal python version we are ok with requiring?
>>
>> I was wondering if people would weigh in discussing that in response to
>> [1] but no one has commented on that part of it.  As another datapoint,
>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
>> [2].
>>
>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048
>
> I for one would be happy to kill off support for anything older than
> 2.6 (which had it's latest release on October 1st, 2008).
>
> Junio, how have we decided in the past which version of x to support?

I do not think there was any conclusion. $gmane/212215 claiming 2.4 support matters for RHEL 5.x users was the last on the topic as far as I can tell, so it boils down to another question: do users on RHEL 5.x matter?

I can read from $gmane/212215 that users of the said platform can safely keep using Python 2.4 under their vendor support contract until 2017. But let's focus on what do these users expect of their system and software they run on it a bit.

When they want to run a piece software that is not shipped with RHEL, either by writing their own or by importing from elsewhere, that needs 2.6 features, what are their options?

 (a) The platform vendor optionally supplies 2.6 with or without
     support;
 (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
     which may even be community-supported, but the vendor does not
     support it; or
 (c) The vendor terminates the support contract for users who choose
     to go (b).

I think we can safely discard (c); if that is the case, the users on the said platform will not choose to update Git either, so it does not matter where the future versions of Git sets the lower bound of Python version at.

If we are not talking about the situation (c), then the users can choose to use 2.6, and more importantly, Python being a popular software, I would imagine that there are reputable sources of prepackaged RPMs for them to do so without going too much hassle of configuring, compiling and installing.

Now how does the decision we make today for releases of Git that haven't yet happened will affect these users? As these versions of newer Git were not shipped with RHEL 5.x, and also I am assuming that Git is a more niche product than Python is, I would imagine that it is very unlikely that the vendor gives it the users as an optional package. The users will have to do the same thing to be able to use such versions of Git as whatever they do in order to use Python 2.6.

Given that, what the vendor originally shipped and officially supports does not affect the choices we would make today for newer versions of Git. The users in a shop where additional third-party software in /usr/local/bin is strictly forbidden, they are stuck with the version of Git that the vendor shipped anyway, because they won't be able to install an updated Git in /usr/local/bin, either.

That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if you are really paranoid, /usr/local/only-for-git/bin/python2.6 where nobody's $PATH points at) is impossible.

So personally I do not think dropping 2.4 is a huge problem for future versions of Git, but I'd like to hear from those working in IT support for large and slow-moving organizations (aka RHEL 5 customers).

Brandon Casey· Jan 25, 2013, 20:23 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

On Wed, Jan 23, 2013 at 12:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 77 quoted lines
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
>> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping <john@keeping.me.uk> wrote:
>>>> When did we last revisit what minimal python version we are ok with requiring?
>>>
>>> I was wondering if people would weigh in discussing that in response to
>>> [1] but no one has commented on that part of it.  As another datapoint,
>>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
>>> [2].
>>>
>>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
>>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048
>>
>> I for one would be happy to kill off support for anything older than
>> 2.6 (which had it's latest release on October 1st, 2008).
>>
>> Junio, how have we decided in the past which version of x to support?
>
> I do not think there was any conclusion.  $gmane/212215 claiming 2.4
> support matters for RHEL 5.x users was the last on the topic as far
> as I can tell, so it boils down to another question: do users on
> RHEL 5.x matter?
>
> I can read from $gmane/212215 that users of the said platform can
> safely keep using Python 2.4 under their vendor support contract
> until 2017.  But let's focus on what do these users expect of their
> system and software they run on it a bit.
>
> When they want to run a piece software that is not shipped with
> RHEL, either by writing their own or by importing from elsewhere,
> that needs 2.6 features, what are their options?
>
>  (a) The platform vendor optionally supplies 2.6 with or without
>      support;
>
>  (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
>      which may even be community-supported, but the vendor does not
>      support it; or
>
>  (c) The vendor terminates the support contract for users who choose
>      to go (b).
>
> I think we can safely discard (c); if that is the case, the users on
> the said platform will not choose to update Git either, so it does
> not matter where the future versions of Git sets the lower bound of
> Python version at.
>
> If we are not talking about the situation (c), then the users can
> choose to use 2.6, and more importantly, Python being a popular
> software, I would imagine that there are reputable sources of
> prepackaged RPMs for them to do so without going too much hassle of
> configuring, compiling and installing.
>
> Now how does the decision we make today for releases of Git that
> haven't yet happened will affect these users?  As these versions of
> newer Git were not shipped with RHEL 5.x, and also I am assuming
> that Git is a more niche product than Python is, I would imagine
> that it is very unlikely that the vendor gives it the users as an
> optional package.  The users will have to do the same thing to be
> able to use such versions of Git as whatever they do in order to use
> Python 2.6.
>
> Given that, what the vendor originally shipped and officially
> supports does not affect the choices we would make today for newer
> versions of Git.  The users in a shop where additional third-party
> software in /usr/local/bin is strictly forbidden, they are stuck
> with the version of Git that the vendor shipped anyway, because they
> won't be able to install an updated Git in /usr/local/bin, either.
>
> That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if
> you are really paranoid, /usr/local/only-for-git/bin/python2.6 where
> nobody's $PATH points at) is impossible.
>
> So personally I do not think dropping 2.4 is a huge problem for
> future versions of Git, but I'd like to hear from those working in
> IT support for large and slow-moving organizations (aka RHEL 5
> customers).

I'm not really in the demographic that you asked to hear from, but I'll give my 2 cents anyway. :)

Firstly, I defer to those with more knowledge and experience with python to decide which version should be the minimum version supported. Python 2.6 seems to be the consensus and that's fine with me.

With respect to older platforms like RHEL 5.X that don't ship with Python 2.6 or later, I suspect most people who work in an organization with a dedicated IT staff can request that a more recent version of python be installed. So, I don't think a python 2.6 requirement (if there was one) would be a blocker for them, and I don't think it would be a major pain for the sysadmin to install.

My only opinion is that if we can avoid breaking older platforms fairly easily, we should do so. If there is someone out there building git packages (e.g. EPEL) for RHEL 5.X or anything else, I imagine that one less dependency makes installing and supporting the package that much easier.

So, my comments shouldn't be taken to suggest that git should support any particular version of python. That decision should be made by those who are willing to support whatever version they feel strongly about.

-Brandon
Erik Faye-Lund· Feb 5, 2013, 16:07 UTC · re: Brandon Casey · lore

Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

On Fri, Jan 25, 2013 at 9:23 PM, Brandon Casey <drafnel@gmail.com> wrote:
Show 94 quoted lines
> On Wed, Jan 23, 2013 at 12:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Sverre Rabbelier <srabbelier@gmail.com> writes:
>>
>>> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping <john@keeping.me.uk> wrote:
>>>>> When did we last revisit what minimal python version we are ok with requiring?
>>>>
>>>> I was wondering if people would weigh in discussing that in response to
>>>> [1] but no one has commented on that part of it.  As another datapoint,
>>>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
>>>> [2].
>>>>
>>>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
>>>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048
>>>
>>> I for one would be happy to kill off support for anything older than
>>> 2.6 (which had it's latest release on October 1st, 2008).
>>>
>>> Junio, how have we decided in the past which version of x to support?
>>
>> I do not think there was any conclusion.  $gmane/212215 claiming 2.4
>> support matters for RHEL 5.x users was the last on the topic as far
>> as I can tell, so it boils down to another question: do users on
>> RHEL 5.x matter?
>>
>> I can read from $gmane/212215 that users of the said platform can
>> safely keep using Python 2.4 under their vendor support contract
>> until 2017.  But let's focus on what do these users expect of their
>> system and software they run on it a bit.
>>
>> When they want to run a piece software that is not shipped with
>> RHEL, either by writing their own or by importing from elsewhere,
>> that needs 2.6 features, what are their options?
>>
>>  (a) The platform vendor optionally supplies 2.6 with or without
>>      support;
>>
>>  (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
>>      which may even be community-supported, but the vendor does not
>>      support it; or
>>
>>  (c) The vendor terminates the support contract for users who choose
>>      to go (b).
>>
>> I think we can safely discard (c); if that is the case, the users on
>> the said platform will not choose to update Git either, so it does
>> not matter where the future versions of Git sets the lower bound of
>> Python version at.
>>
>> If we are not talking about the situation (c), then the users can
>> choose to use 2.6, and more importantly, Python being a popular
>> software, I would imagine that there are reputable sources of
>> prepackaged RPMs for them to do so without going too much hassle of
>> configuring, compiling and installing.
>>
>> Now how does the decision we make today for releases of Git that
>> haven't yet happened will affect these users?  As these versions of
>> newer Git were not shipped with RHEL 5.x, and also I am assuming
>> that Git is a more niche product than Python is, I would imagine
>> that it is very unlikely that the vendor gives it the users as an
>> optional package.  The users will have to do the same thing to be
>> able to use such versions of Git as whatever they do in order to use
>> Python 2.6.
>>
>> Given that, what the vendor originally shipped and officially
>> supports does not affect the choices we would make today for newer
>> versions of Git.  The users in a shop where additional third-party
>> software in /usr/local/bin is strictly forbidden, they are stuck
>> with the version of Git that the vendor shipped anyway, because they
>> won't be able to install an updated Git in /usr/local/bin, either.
>>
>> That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if
>> you are really paranoid, /usr/local/only-for-git/bin/python2.6 where
>> nobody's $PATH points at) is impossible.
>>
>> So personally I do not think dropping 2.4 is a huge problem for
>> future versions of Git, but I'd like to hear from those working in
>> IT support for large and slow-moving organizations (aka RHEL 5
>> customers).
>
> I'm not really in the demographic that you asked to hear from, but
> I'll give my 2 cents anyway. :)
>
> Firstly, I defer to those with more knowledge and experience with
> python to decide which version should be the minimum version
> supported.  Python 2.6 seems to be the consensus and that's fine with
> me.
>
> With respect to older platforms like RHEL 5.X that don't ship with
> Python 2.6 or later, I suspect most people who work in an organization
> with a dedicated IT staff can request that a more recent version of
> python be installed.  So, I don't think a python 2.6 requirement (if
> there was one) would be a blocker for them, and I don't think it would
> be a major pain for the sysadmin to install.
>

Just a datapoint: I'm working with customers on RHEL 5.X that unfortunately has an extremely lengthy (>3 months) process of approving non-standard packages for install. Yeah, it's horrible, but some times that's reality.

We are currently not using Git with that client, but we are in the process of changing that. Said customer already have an exception for all versions of Git.

I doubt this will end up being a problem in reality or not, but if it will be, I'm sure it can be worked around out. I'm just pointing out that the above suspicion might not be accurate.

John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 3/8] git_remote_helpers: Force rebuild if python version changes

When different version of python are used to build via distutils, the behaviour can change. Detect changes in version and pass --force in this case.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git_remote_helpers/.gitignore | 1 +
 git_remote_helpers/Makefile   | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)
Show changes to 2 files +8 −1

git_remote_helpers/.gitignore, git_remote_helpers/Makefile

diff --git a/git_remote_helpers/.gitignore b/git_remote_helpers/.gitignore
index 2247d5f..06c664f 100644
--- a/git_remote_helpers/.gitignore
+++ b/git_remote_helpers/.gitignore
@@ -1,2 +1,3 @@
+/GIT-PYTHON_VERSION
 /build
 /dist
diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
index f65f064..91f458f 100644
--- a/git_remote_helpers/Makefile
+++ b/git_remote_helpers/Makefile
@@ -25,8 +25,14 @@ PYLIBDIR=$(shell $(PYTHON_PATH) -c \
 	 "import sys; \
 	 print('lib/python%i.%i/site-packages' % sys.version_info[:2])")
 
+py_version=$(shell $(PYTHON_PATH) -c \
+	'import sys; print("%i.%i" % sys.version_info[:2])')
+
 all: $(pysetupfile)
-	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
+	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
+	flags=--force; \
+	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
+	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
 
 install: $(pysetupfile)
 	$(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix)
-- 
1.8.1.353.gc992d5a.dirty
Sverre Rabbelier· Jan 23, 2013, 18:51 UTC · re: John Keeping · lore

Re: [PATCH v3 3/8] git_remote_helpers: Force rebuild if python version changes

On Sun, Jan 20, 2013 at 5:15 AM, John Keeping <john@keeping.me.uk> wrote:
Show 5 quoted lines
> When different version of python are used to build via distutils, the
> behaviour can change.  Detect changes in version and pass --force in
> this case.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>

Someone else's review on this would be appreciated, the idea sounds sane but I can't really comment on the implementation.

-- Cheers,

Sverre Rabbelier
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 4/8] git_remote_helpers: Use 2to3 if building with Python 3

Using the approach detailed in the Python documentation[1], run 2to3 on the code as part of the build if building with Python 3.

The code itself requires no changes to convert cleanly.
[1] http://docs.python.org/3.3/howto/pyporting.html#during-installation
Signed-off-by: John Keeping <john@keeping.me.uk>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
---
On Fri, 18 Jan 2013 23:52:16 -0800, Sverre Rabbelier wrote:
> Assuming you tried this out on both 2.x and 3.x:
>
> Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
I ran the test suite with Python 2.7.3 and 3.2.3.
 git_remote_helpers/setup.py | 10 ++++++++++
 1 file changed, 10 insertions(+)
Show changes to git_remote_helpers/setup.py +10 −0
diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
index 4d434b6..6de41de 100644
--- a/git_remote_helpers/setup.py
+++ b/git_remote_helpers/setup.py
@@ -4,6 +4,15 @@
 
 from distutils.core import setup
 
+# If building under Python3 we need to run 2to3 on the code, do this by
+# trying to import distutils' 2to3 builder, which is only available in
+# Python3.
+try:
+    from distutils.command.build_py import build_py_2to3 as build_py
+except ImportError:
+    # 2.x
+    from distutils.command.build_py import build_py
+
 setup(
     name = 'git_remote_helpers',
     version = '0.1.0',
@@ -14,4 +23,5 @@ setup(
     url = 'http://www.git-scm.com/',
     package_dir = {'git_remote_helpers': ''},
     packages = ['git_remote_helpers', 'git_remote_helpers.git'],
+    cmdclass = {'build_py': build_py},
 )
-- 
1.8.1.353.gc992d5a.dirty
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 5/8] svn-fe: allow svnrdump_sim.py to run with Python 3

The changes to allow this script to run with Python 3 are minimal and do not affect its functionality on the versions of Python 2 that are already supported (2.4 onwards).

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 contrib/svn-fe/svnrdump_sim.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/svn-fe/svnrdump_sim.py +2 −2
diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 17cf6f9..4e78a1c 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -14,7 +14,7 @@ if sys.hexversion < 0x02040000:
 
 def getrevlimit():
         var = 'SVNRMAX'
-        if os.environ.has_key(var):
+        if var in os.environ:
                 return os.environ[var]
         return None
 
@@ -44,7 +44,7 @@ def writedump(url, lower, upper):
 
 if __name__ == "__main__":
         if not (len(sys.argv) in (3, 4, 5)):
-                print "usage: %s dump URL -rLOWER:UPPER"
+                print("usage: %s dump URL -rLOWER:UPPER")
                 sys.exit(1)
         if not sys.argv[1] == 'dump': raise NotImplementedError('only "dump" is suppported.')
         url = sys.argv[2]
-- 
1.8.1.353.gc992d5a.dirty
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[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 <john@keeping.me.uk>
---
 git-remote-testpy.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to git-remote-testpy.py +4 −4
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 < 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('hex'))
     repo.hash = hasher.hexdigest()
 
     repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1.353.gc992d5a.dirty
John Keeping· Jan 26, 2013, 17:51 UTC · re: John Keeping · lore

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 <john@keeping.me.uk>
---
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(-)
Show changes to git-remote-testpy.py +4 −4
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(
-- 
1.8.1.1
Junio C Hamano· Jan 26, 2013, 21:44 UTC · re: John Keeping · lore

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

John Keeping <john@keeping.me.uk> writes:
Show 9 quoted lines
> 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.
Show 30 quoted lines
>
>  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(
John Keeping· Jan 26, 2013, 23:32 UTC · re: Junio C Hamano · lore

[PATCH] git-remote-testpy: fix patch hashing on Python 3

When this change was originally made (0846b0c - git-remote-testpy: hash bytes explicitly , I didn't realised that the "hex" encoding we chose 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, UTF-8 really is the best option here.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Sat, Jan 26, 2013 at 01:44:55PM -0800, Junio C Hamano wrote:
> 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.
Here it is, based on next obviously.
 git-remote-testpy.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to git-remote-testpy.py +1 −1
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..4713363 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -45,7 +45,7 @@ def get_repo(alias, url):
     repo.get_head()
 
     hasher = _digest()
-    hasher.update(repo.path.encode('hex'))
+    hasher.update(repo.path.encode('utf-8'))
     repo.hash = hasher.hexdigest()
 
     repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1.1
Michael Haggerty· Jan 27, 2013, 04:44 UTC · re: Junio C Hamano · lore

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

On 01/26/2013 10:44 PM, Junio C Hamano wrote:
Show 48 quoted lines
> 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/
Junio C Hamano· Jan 27, 2013, 05:30 UTC · re: Michael Haggerty · lore

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

Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 6 quoted lines
> 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(-)
Show changes to git-remote-testgit.py +1 −1
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(
John Keeping· Jan 27, 2013, 14:21 UTC · re: Junio C Hamano · lore

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:
Show 15 quoted lines
> Michael Haggerty <mhagger@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
Sverre Rabbelier· Jan 27, 2013, 05:30 UTC · re: Michael Haggerty · lore

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

On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty <mhagger@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
Michael Haggerty· Jan 27, 2013, 08:41 UTC · re: Sverre Rabbelier · lore

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

On 01/27/2013 06:30 AM, Sverre Rabbelier wrote:
Show 7 quoted lines
> On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty <mhagger@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 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
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
John Keeping· Jan 27, 2013, 14:13 UTC · re: Michael Haggerty · lore

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

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
John Keeping· Jan 27, 2013, 14:50 UTC · re: John Keeping · lore

[PATCH] git-remote-testpy: fix patch hashing on Python 3

When this change was originally made (0846b0c - git-remote-testpy: hash bytes explicitly , I didn't realised that the "hex" encoding we chose is a "bytes to bytes" encoding so it just fails with an error on Python 3 in the same way as the original code.

It is not possible to provide a single code path that works on Python 2 and Python 3 since Python 2.x will attempt to decode the string before encoding it, which fails for strings that are not valid in the default encoding. Python 3.1 introduced the "surrogateescape" error handler which handles this correctly and permits a bytes -> unicode -> bytes round-trip to be lossless.

At this point Python 3.0 is unsupported so we don't go out of our way to try to support it.

Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Sun, Jan 27, 2013 at 02:13:29PM +0000, John Keeping wrote:
Show 18 quoted lines
> On Sun, Jan 27, 2013 at 05:44:37AM +0100, 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:
> > 
> >     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()
How about this?
 git-remote-testpy.py | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Show changes to git-remote-testpy.py +17 −1
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..16b0c52 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
     sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
     sys.exit(1)
 
+
+def _encode_filepath(path):
+    """Encodes a Unicode file path to a byte string.
+
+    On Python 2 this is a no-op; on Python 3 we encode the string as
+    suggested by [1] which allows an exact round-trip from the command line
+    to the filesystem.
+
+    [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
+
+    """
+    if sys.hexversion < 0x03000000:
+        return path
+    return path.encode('utf-8', 'surrogateescape')
+
+
 def get_repo(alias, url):
     """Returns a git repository object initialized for usage.
     """
@@ -45,7 +61,7 @@ def get_repo(alias, url):
     repo.get_head()
 
     hasher = _digest()
-    hasher.update(repo.path.encode('hex'))
+    hasher.update(_encode_filepath(repo.path))
     repo.hash = hasher.hexdigest()
 
     repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1.1
Junio C Hamano· Jan 27, 2013, 19:49 UTC · re: John Keeping · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

John Keeping <john@keeping.me.uk> writes:
Show 18 quoted lines
> When this change was originally made (0846b0c - git-remote-testpy: hash
> bytes explicitly , I didn't realised that the "hex" encoding we chose is
> a "bytes to bytes" encoding so it just fails with an error on Python 3
> in the same way as the original code.
>
> It is not possible to provide a single code path that works on Python 2
> and Python 3 since Python 2.x will attempt to decode the string before
> encoding it, which fails for strings that are not valid in the default
> encoding.  Python 3.1 introduced the "surrogateescape" error handler
> which handles this correctly and permits a bytes -> unicode -> bytes
> round-trip to be lossless.
>
> At this point Python 3.0 is unsupported so we don't go out of our way to
> try to support it.
>
> Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
Thanks; will queue and wait for an Ack from Michael.

Does the helper function need to be named with leading underscore, though?

Show 61 quoted lines
> On Sun, Jan 27, 2013 at 02:13:29PM +0000, John Keeping wrote:
>> On Sun, Jan 27, 2013 at 05:44:37AM +0100, 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:
>> > 
>> >     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()
>
> How about this?
>
>  git-remote-testpy.py | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index c7a04ec..16b0c52 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
>      sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
>      sys.exit(1)
>  
> +
> +def _encode_filepath(path):
> +    """Encodes a Unicode file path to a byte string.
> +
> +    On Python 2 this is a no-op; on Python 3 we encode the string as
> +    suggested by [1] which allows an exact round-trip from the command line
> +    to the filesystem.
> +
> +    [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
> +
> +    """
> +    if sys.hexversion < 0x03000000:
> +        return path
> +    return path.encode('utf-8', 'surrogateescape')
> +
> +
>  def get_repo(alias, url):
>      """Returns a git repository object initialized for usage.
>      """
> @@ -45,7 +61,7 @@ def get_repo(alias, url):
>      repo.get_head()
>  
>      hasher = _digest()
> -    hasher.update(repo.path.encode('hex'))
> +    hasher.update(_encode_filepath(repo.path))
>      repo.hash = hasher.hexdigest()
>  
>      repo.get_base_path = lambda base: os.path.join(
John Keeping· Jan 27, 2013, 20:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

On Sun, Jan 27, 2013 at 11:49:39AM -0800, Junio C Hamano wrote:
Show 25 quoted lines
> John Keeping <john@keeping.me.uk> writes:
> 
> > When this change was originally made (0846b0c - git-remote-testpy: hash
> > bytes explicitly , I didn't realised that the "hex" encoding we chose is
> > a "bytes to bytes" encoding so it just fails with an error on Python 3
> > in the same way as the original code.
> >
> > It is not possible to provide a single code path that works on Python 2
> > and Python 3 since Python 2.x will attempt to decode the string before
> > encoding it, which fails for strings that are not valid in the default
> > encoding.  Python 3.1 introduced the "surrogateescape" error handler
> > which handles this correctly and permits a bytes -> unicode -> bytes
> > round-trip to be lossless.
> >
> > At this point Python 3.0 is unsupported so we don't go out of our way to
> > try to support it.
> >
> > Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> > ---
> 
> Thanks; will queue and wait for an Ack from Michael.
> 
> Does the helper function need to be named with leading underscore,
> though?

It's a Python convention for internal functions. Since this is a script not a library module I don't feel strongly about it in this case.

John
Junio C Hamano· Jan 27, 2013, 20:11 UTC · re: John Keeping · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

John Keeping <john@keeping.me.uk> writes:
Show 7 quoted lines
>> Thanks; will queue and wait for an Ack from Michael.
>> 
>> Does the helper function need to be named with leading underscore,
>> though?
>
> ...  Since this is a script
> not a library module I don't feel strongly about it in this case.
That is exactly why I asked.
John Keeping· Jan 27, 2013, 20:21 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote:
Show 11 quoted lines
> John Keeping <john@keeping.me.uk> writes:
> 
> >> Thanks; will queue and wait for an Ack from Michael.
> >> 
> >> Does the helper function need to be named with leading underscore,
> >> though?
> >
> > ...  Since this is a script
> > not a library module I don't feel strongly about it in this case.
> 
> That is exactly why I asked.

So I think the answer is "habit, but I probably shouldn't have put it in in this case".

John
Junio C Hamano· Jan 27, 2013, 20:38 UTC · re: John Keeping · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

John Keeping <john@keeping.me.uk> writes:
Show 15 quoted lines
> On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>> 
>> >> Thanks; will queue and wait for an Ack from Michael.
>> >> 
>> >> Does the helper function need to be named with leading underscore,
>> >> though?
>> >
>> > ...  Since this is a script
>> > not a library module I don't feel strongly about it in this case.
>> 
>> That is exactly why I asked.
>
> So I think the answer is "habit, but I probably shouldn't have put it
> in in this case".

OK, then I'll queue with a local amend to drop the leading underscore.

Thanks.
Junio C Hamano· Jan 27, 2013, 20:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> John Keeping <john@keeping.me.uk> writes:
>
>> So I think the answer is "habit, but I probably shouldn't have put it
>> in in this case".
>
> OK, then I'll queue with a local amend to drop the leading
> underscore.

So this is what I will be queuing (I'd appreciate the second set of eyes, though), with the leading-underscore removal and log message typofixes.

I remember that I earlier asked somewhere if we want to say "Python 3.x that is older than 3.y is unsupported"

    http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
but I was told that we will support all versions in 3.x series, IIRC.

Does this patch contradict with that? If so I think we would need to revisit the update to CodingGuidelines in that thread.

I am perfectly fine with discarding early 3.x as "0.x releases of Python3", but I would want to see our document say so if that is what we do.

-- >8 --
From: John Keeping <john@keeping.me.uk>
Date: Sun, 27 Jan 2013 14:50:56 +0000
Subject: [PATCH] git-remote-testpy: fix path hashing on Python 3

When this change was originally made (0846b0c - git-remote-testpy: hash bytes explicitly , I didn't realise that the "hex" encoding we chose is a "bytes to bytes" encoding so it just fails with an error on Python 3 in the same way as the original code.

It is not possible to provide a single code path that works on Python 2 and Python 3 since Python 2.x will attempt to decode the string before encoding it, which fails for strings that are not valid in the default encoding. Python 3.1 introduced the "surrogateescape" error handler which handles this correctly and permits a bytes -> unicode -> bytes round-trip to be lossless.

At this point Python 3.0 is unsupported so we don't go out of our way to try to support it.

Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-remote-testpy.py | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Show changes to git-remote-testpy.py +17 −1
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..6098bdd 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
     sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
     sys.exit(1)
 
+
+def encode_filepath(path):
+    """Encodes a Unicode file path to a byte string.
+
+    On Python 2 this is a no-op; on Python 3 we encode the string as
+    suggested by [1] which allows an exact round-trip from the command line
+    to the filesystem.
+
+    [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
+
+    """
+    if sys.hexversion < 0x03000000:
+        return path
+    return path.encode('utf-8', 'surrogateescape')
+
+
 def get_repo(alias, url):
     """Returns a git repository object initialized for usage.
     """
@@ -45,7 +61,7 @@ def get_repo(alias, url):
     repo.get_head()
 
     hasher = _digest()
-    hasher.update(repo.path.encode('hex'))
+    hasher.update(encode_filepath(repo.path))
     repo.hash = hasher.hexdigest()
 
     repo.get_base_path = lambda base: os.path.join(
-- 
1.8.1.1.550.g40037fd
John Keeping· Jan 27, 2013, 22:42 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

On Sun, Jan 27, 2013 at 12:47:09PM -0800, Junio C Hamano wrote:
Show 9 quoted lines
> I remember that I earlier asked somewhere if we want to say "Python
> 3.x that is older than 3.y is unsupported"
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
> 
> but I was told that we will support all versions in 3.x series, IIRC.
> 
> Does this patch contradict with that?  If so I think we would need
> to revisit the update to CodingGuidelines in that thread.
Yes.  I'll send an update to that over the next couple of days.

I think 3.1 and later is fine, when I said "Python 3.0 is unsupported" in the commit message below, I meant "unsupported by the Python developers". Support ended at least 3 months ago:

    http://hg.python.org/peps/rev/6d2e9d41dfaa
John
Junio C Hamano· Jan 27, 2013, 23:18 UTC · re: John Keeping · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

John Keeping <john@keeping.me.uk> writes:
Show 16 quoted lines
> On Sun, Jan 27, 2013 at 12:47:09PM -0800, Junio C Hamano wrote:
>> I remember that I earlier asked somewhere if we want to say "Python
>> 3.x that is older than 3.y is unsupported"
>> 
>>     http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
>> 
>> but I was told that we will support all versions in 3.x series, IIRC.
>> 
>> Does this patch contradict with that?  If so I think we would need
>> to revisit the update to CodingGuidelines in that thread.
>
> Yes.  I'll send an update to that over the next couple of days.
>
> I think 3.1 and later is fine, when I said "Python 3.0 is unsupported"
> in the commit message below, I meant "unsupported by the Python
> developers".

Yeah, I knew what you meant. I do not think it is so wrong to write 3.0 off as an early 0.x release of Python3 that was not yet usable for that exact reason.

Michael Haggerty· Jan 28, 2013, 10:44 UTC · re: John Keeping · lore

Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3

On 01/27/2013 03:50 PM, John Keeping wrote:
Show 80 quoted lines
> When this change was originally made (0846b0c - git-remote-testpy: hash
> bytes explicitly , I didn't realised that the "hex" encoding we chose is
> a "bytes to bytes" encoding so it just fails with an error on Python 3
> in the same way as the original code.
> 
> It is not possible to provide a single code path that works on Python 2
> and Python 3 since Python 2.x will attempt to decode the string before
> encoding it, which fails for strings that are not valid in the default
> encoding.  Python 3.1 introduced the "surrogateescape" error handler
> which handles this correctly and permits a bytes -> unicode -> bytes
> round-trip to be lossless.
> 
> At this point Python 3.0 is unsupported so we don't go out of our way to
> try to support it.
> 
> Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
> On Sun, Jan 27, 2013 at 02:13:29PM +0000, John Keeping wrote:
>> On Sun, Jan 27, 2013 at 05:44:37AM +0100, 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:
>>>
>>>     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()
> 
> How about this?
> 
>  git-remote-testpy.py | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index c7a04ec..16b0c52 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
>      sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
>      sys.exit(1)
>  
> +
> +def _encode_filepath(path):
> +    """Encodes a Unicode file path to a byte string.
> +
> +    On Python 2 this is a no-op; on Python 3 we encode the string as
> +    suggested by [1] which allows an exact round-trip from the command line
> +    to the filesystem.
> +
> +    [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
> +
> +    """
> +    if sys.hexversion < 0x03000000:
> +        return path
> +    return path.encode('utf-8', 'surrogateescape')
> +
> +
>  def get_repo(alias, url):
>      """Returns a git repository object initialized for usage.
>      """
> @@ -45,7 +61,7 @@ def get_repo(alias, url):
>      repo.get_head()
>  
>      hasher = _digest()
> -    hasher.update(repo.path.encode('hex'))
> +    hasher.update(_encode_filepath(repo.path))
>      repo.hash = hasher.hexdigest()
>  
>      repo.get_base_path = lambda base: os.path.join(
> 

NAK. It is still not right. If the locale is not utf-8 based, then it is incorrect to re-encode the string using utf-8. I think you really have to use sys.getfilesystemencoding() as I suggested.

The attached program demonstrates the problem: the output of re-encoding using UTF-8 depends on the locale, whereas that of re-encoding using the filesystemencoding is independent of locale (as we want). The output, using Python 3.2.3:

# This is 0xb6 0xc3: $ ARG="ö" $ LANG='C' /usr/bin/python3 chaos3.py "$ARG" LANG = 'C' fse = 'ascii' sys.argv[1] = u"U+DCC3 U+DCB6" re-encoded using UTF-8: b"C3 B6" re-encoded using fse: b"C3 B6"

$ LANG='C.UTF-8' /usr/bin/python3 chaos3.py "$ARG" LANG = 'C.UTF-8' fse = 'utf-8' sys.argv[1] = u"U+00F6" re-encoded using UTF-8: b"C3 B6" re-encoded using fse: b"C3 B6"

$ LANG='en_US.iso88591' /usr/bin/python3 chaos3.py "$ARG" LANG = 'en_US.iso88591' fse = 'iso8859-1' sys.argv[1] = u"U+00C3 U+00B6" re-encoded using UTF-8: b"C3 83 C2 B6" re-encoded using fse: b"C3 B6"

Even though the Unicode intermediate representation is different for UTF-8 and ASCII, re-encoding using the correct encoding gives back the original bytes (which is what we want). But when using the ios8859-1 locale, the original bytes look like a valid latin1 string so they are not surrogated going in, giving the incorrect Unicode string u"U+00C3 U+00B6". When this is re-encoded using UTF-8, the code points U+00C3 and U+00B6 are each encoded as two bytes.

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


#! /usr/bin/python3

import sys
import os


def explicit(s):
    """Convert a string or bytestring into an unambiguous human-readable string."""

    if isinstance(s, str):
        return 'u"%s"' % (' '.join('U+%04X' % (ord(c),) for c in s))
    else:
        return 'b"%s"' % (' '.join('%02X' % (c,) for c in s))


fse = sys.getfilesystemencoding()

print('LANG = %r' % (os.getenv('LANG'),))
print('fse = %r' % (fse,))
print('sys.argv[1] = %s' % explicit(sys.argv[1]))
print('re-encoded using UTF-8: %s' % explicit(sys.argv[1].encode('utf-8', 'surrogateescape')))
print('re-encoded using fse: %s' % explicit(sys.argv[1].encode(fse, 'surrogateescape')))
print()
John Keeping· Jan 28, 2013, 11:20 UTC · re: Michael Haggerty · lore

[PATCH] fixup! git-remote-testpy: fix path hashing on Python 3

--- On Mon, Jan 28, 2013 at 11:44:34AM +0100, Michael Haggerty wrote:

> NAK.  It is still not right.  If the locale is not utf-8 based, then it
> is incorrect to re-encode the string using utf-8.  I think you really
> have to use sys.getfilesystemencoding() as I suggested.

If you'd asked me what the patch contained I would have said it did use getfilesystemencoding(), but I can't disbelieve my own eyes :-(

Junio, please can you squash this in?
 git-remote-testpy.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to git-remote-testpy.py +1 −1
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index 6098bdd..ca67899 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -49,7 +49,7 @@ def encode_filepath(path):
     """
     if sys.hexversion < 0x03000000:
         return path
-    return path.encode('utf-8', 'surrogateescape')
+    return path.encode(sys.getfilesystemencoding(), 'surrogateescape')
 
 
 def get_repo(alias, url):
-- 
1.8.1.1
Junio C Hamano· Jan 28, 2013, 17:53 UTC · re: John Keeping · lore

Re: [PATCH] fixup! git-remote-testpy: fix path hashing on Python 3

John Keeping <john@keeping.me.uk> writes:
Show 10 quoted lines
> ---
> On Mon, Jan 28, 2013 at 11:44:34AM +0100, Michael Haggerty wrote:
>> NAK.  It is still not right.  If the locale is not utf-8 based, then it
>> is incorrect to re-encode the string using utf-8.  I think you really
>> have to use sys.getfilesystemencoding() as I suggested.
>
> If you'd asked me what the patch contained I would have said it did use
> getfilesystemencoding(), but I can't disbelieve my own eyes :-(
>
> Junio, please can you squash this in?

Sure. Thanks for double-checking, Michael. I knew there was something missing but I didn't spot the difference myself.

Show 16 quoted lines
>  git-remote-testpy.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index 6098bdd..ca67899 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -49,7 +49,7 @@ def encode_filepath(path):
>      """
>      if sys.hexversion < 0x03000000:
>          return path
> -    return path.encode('utf-8', 'surrogateescape')
> +    return path.encode(sys.getfilesystemencoding(), 'surrogateescape')
>  
>  
>  def get_repo(alias, url):
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 7/8] git-remote-testpy: don't do unbuffered text I/O

Python 3 forbids unbuffered I/O in text mode. Change the reading of stdin in git-remote-testpy so that we read the lines as bytes and then decode them a line at a time.

This allows us to keep the I/O unbuffered in order to avoid reintroducing the bug fixed by commit 7fb8e16 (git-remote-testgit: fix race when spawning fast-import).

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 git-remote-testpy.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
Show changes to git-remote-testpy.py +7 −3
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index 197b7be..5dbf1cc 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -154,7 +154,7 @@ def do_import(repo, args):
     refs = [ref]
 
     while True:
-        line = sys.stdin.readline()
+        line = sys.stdin.readline().decode()
         if line == '\n':
             break
         if not line.startswith('import '):
@@ -225,7 +225,7 @@ def read_one_line(repo):
 
     line = sys.stdin.readline()
 
-    cmdline = line
+    cmdline = line.decode()
 
     if not cmdline:
         warn("Unexpected EOF")
@@ -277,7 +277,11 @@ def main(args):
 
     more = True
 
-    sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)
+    # Use binary mode since Python 3 does not permit unbuffered I/O in text
+    # mode.  Unbuffered I/O is required to avoid data that should be going
+    # to git-fast-import after an "export" command getting caught in our
+    # stdin buffer instead.
+    sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', 0)
     while (more):
         more = read_one_line(repo)
 
-- 
1.8.1.353.gc992d5a.dirty
John Keeping· Jan 20, 2013, 13:15 UTC · re: John Keeping · lore

[PATCH v3 8/8] git-remote-testpy: call print as a function

This is harmless in Python 2, which sees the parentheses as redundant grouping, but is required for Python 3. Since this is the only change required to make this script just run under Python 3 without needing 2to3 it seems worthwhile.

The case of an empty print must be handled specially because in that case Python 2 will interpret '()' as an empty tuple and print it as '()'; inserting an empty string fixes this.

Signed-off-by: John Keeping <john@keeping.me.uk>
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 git-remote-testpy.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
Show changes to git-remote-testpy.py +14 −14
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index 5dbf1cc..c7a04ec 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -87,9 +87,9 @@ def do_capabilities(repo, args):
     """Prints the supported capabilities.
     """
 
-    print "import"
-    print "export"
-    print "refspec refs/heads/*:%s*" % repo.prefix
+    print("import")
+    print("export")
+    print("refspec refs/heads/*:%s*" % repo.prefix)
 
     dirname = repo.get_base_path(repo.gitdir)
 
@@ -98,11 +98,11 @@ def do_capabilities(repo, args):
 
     path = os.path.join(dirname, 'git.marks')
 
-    print "*export-marks %s" % path
+    print("*export-marks %s" % path)
     if os.path.exists(path):
-        print "*import-marks %s" % path
+        print("*import-marks %s" % path)
 
-    print # end capabilities
+    print('') # end capabilities
 
 
 def do_list(repo, args):
@@ -115,16 +115,16 @@ def do_list(repo, args):
 
     for ref in repo.revs:
         debug("? refs/heads/%s", ref)
-        print "? refs/heads/%s" % ref
+        print("? refs/heads/%s" % ref)
 
     if repo.head:
         debug("@refs/heads/%s HEAD" % repo.head)
-        print "@refs/heads/%s HEAD" % repo.head
+        print("@refs/heads/%s HEAD" % repo.head)
     else:
         debug("@refs/heads/master HEAD")
-        print "@refs/heads/master HEAD"
+        print("@refs/heads/master HEAD")
 
-    print # end list
+    print('') # end list
 
 
 def update_local_repo(repo):
@@ -164,7 +164,7 @@ def do_import(repo, args):
         ref = line[7:].strip()
         refs.append(ref)
 
-    print "feature done"
+    print("feature done")
 
     if os.environ.get("GIT_REMOTE_TESTGIT_FAILURE"):
         die('Told to fail')
@@ -172,7 +172,7 @@ def do_import(repo, args):
     repo = update_local_repo(repo)
     repo.exporter.export_repo(repo.gitdir, refs)
 
-    print "done"
+    print("done")
 
 
 def do_export(repo, args):
@@ -192,8 +192,8 @@ def do_export(repo, args):
         repo.non_local.push(repo.gitdir)
 
     for ref in changed:
-        print "ok %s" % ref
-    print
+        print("ok %s" % ref)
+    print('')
 
 
 COMMANDS = {
-- 
1.8.1.353.gc992d5a.dirty

← back to recent threads