You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Petyovský Petr <pe...@feec.vutbr.cz> on 2020/06/11 11:42:03 UTC

Python3 support for hot-backup.py

Hi developers.

I upgraded my server to the fedora core 32 which completely drop Python2.

So I prepare some quick and dirty patch (I really dont know Python at 
all) which makes hot-backup.py script working again under Python3 
environment.

Maybe some real python programmer is able improve my patch and include
all fixes to the trunk.

Regards,
Peter

-- 
Ing. Petr Petyovsky, Ph.D.

Brno University of Technology
Faculty of Electrical Engineering and Communication
Department of Control and Instrumentation
Technicka 3082/12, 616 00 Brno
Czech Republic
European Union

Re: Python3 support for hot-backup.py

Posted by Nathan Hartman <ha...@gmail.com>.
On Mon, Jun 15, 2020 at 4:59 AM Branko Čibej <br...@apache.org> wrote:

> On 15.06.2020 10:51, Petyovský Petr wrote:
>
> Hi there.
>
> Is there someone who is reponsible for the hot-backup.py script, that here
> answer me?
>
> Or I can create feature request on the subversion Apache Jira directly
> without discussion here?
>
> Thanks for reactions.
>
>
> A feature request in Jira will most likely be ignored. A patch for the
> script sent to this list will most likely be accepted.
>

Peter sent a patch in the OP on 11 June.

It looks reasonable to me, but I'd rather if someone more experienced in
Python 2 -> 3 conversion could review.

Nathan

Re: Python3 support for hot-backup.py

Posted by Branko Čibej <br...@apache.org>.
On 15.06.2020 10:51, Petyovský Petr wrote:
> Hi there.
>
> Is there someone who is reponsible for the hot-backup.py script, that
> here answer me?
>
> Or I can create feature request on the subversion Apache Jira directly
> without discussion here?
>
> Thanks for reactions.

A feature request in Jira will most likely be ignored. A patch for the
script sent to this list will most likely be accepted.

-- Brane


> Dne 11.6.2020 v 13:42 Petyovský Petr napsal(a):
>> Hi developers.
>>
>> I upgraded my server to the fedora core 32 which completely drop
>> Python2.
>>
>> So I prepare some quick and dirty patch (I really dont know Python at
>> all) which makes hot-backup.py script working again under Python3
>> environment.
>>
>> Maybe some real python programmer is able improve my patch and include
>> all fixes to the trunk.
>>
>> Regards,
>> Peter
>
>


Re: Python3 support for hot-backup.py

Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2020/06/19 22:17, Yasuhito FUTATSUKI wrote:
> On 2020/06/16 2:09, Yasuhito FUTATSUKI wrote:
>> On 2020/06/16 0:18, C. Michael Pilato wrote:
>>> On Mon, Jun 15, 2020 at 10:32 AM Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
>>> wrote:
> 
>>> As for your question:  if I force "svnlook" to create errors (by setting
>>> the svnlook variable to "/usr/bin/svn"), today I see an error message with
>>> the b'...' formatting.  Adding .decode() as you suggested makes the b'...'
>>> go away and I see what I'd expect to see.  As far as I can tell, I'm using
>>> the "en-US.UTF-8" locale, though -- not the "C" one.  But maybe I'm just
>>> getting lucky because the locale encoding is UTF-8 and not, say, Shift-JIS
>>> or something?  I dunno.
>>
>> I confirmed that the code with .decode() work well in ja_JP.UTF-8 locale
>> on Python 3.6 and Python 3.7 (but I got an error like "'ascii' codec can't
>> decode byte 0xe3 in position 18: ordinal not in range(128)").
>                                                             ^ on Python 2.7
>  
>> With non UTF-8, non ascii locale, .decode() without specifying encoding
>> causes UnicodeDecodeError on 'utf-8' codecs on Python 3.6 and 3.7.
>> So if we want to allow this script run on non UTF-8, non ascii locale
>> with Python 3, it needs additional code to set encoding.
> 
> I wrote "additional code".
> 
> [[[
> Index: tools/backup/hot-backup.py.in
> ===================================================================
> --- tools/backup/hot-backup.py.in	(revision 1878988)
> +++ tools/backup/hot-backup.py.in	(working copy)
> @@ -36,6 +36,7 @@
>  ######################################################################
>  
>  import sys, os, getopt, stat, re, time, shutil, subprocess
> +import locale
>  import functools
>  
>  ######################################################################
> @@ -204,10 +205,18 @@
>    """Examine the repository REPO_DIR using the svnlook binary
>    specified by SVNLOOK, and return the youngest revision."""
>  
> -  p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
> -                       stdin=subprocess.PIPE,
> -                       stdout=subprocess.PIPE,
> -                       stderr=subprocess.PIPE)
> +  if b'' == '':
> +    p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
> +                         stdin=subprocess.PIPE,
> +                         stdout=subprocess.PIPE,
> +                         stderr=subprocess.PIPE)
> +  else:
> +    p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
> +                         stdin=subprocess.PIPE,
> +                         stdout=subprocess.PIPE,
> +                         stderr=subprocess.PIPE,
> +                         errors='backslashreplace', # foolproof
> +                         encoding=locale.getpreferredencoding())
>    infile, outfile, errfile = p.stdin, p.stdout, p.stderr
>  
>    stdout_lines = outfile.readlines()
> @@ -220,7 +229,7 @@
>      raise Exception("Unable to find the youngest revision for repository '%s'"
>                      ": %s" % (repo_dir, stderr_lines[0].rstrip()))
>  
> -  return stdout_lines[0].strip().decode()
> +  return stdout_lines[0].strip()
>  
>  ######################################################################
>  # Main
> ]]]
> 
> I confirmed it works on Python 2.7, Python 3.7 on FreeBSD with
> ja_JP.eucJP, ja_JP.UTF-8 locale. However I'm not sure if it works or not
> on Windows, especially non English environment. It depends on that
> the encoding of the error message of SVNLOOK equals to the result of 
> locale.getreferredencoding() here.

Although I couldn't confirm it on Windows (because I couldn't set up
gettext/libintl for x64), I commited it in r1882780.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>

Re: Python3 support for hot-backup.py

Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2020/06/16 2:09, Yasuhito FUTATSUKI wrote:
> On 2020/06/16 0:18, C. Michael Pilato wrote:
>> On Mon, Jun 15, 2020 at 10:32 AM Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
>> wrote:

>> As for your question:  if I force "svnlook" to create errors (by setting
>> the svnlook variable to "/usr/bin/svn"), today I see an error message with
>> the b'...' formatting.  Adding .decode() as you suggested makes the b'...'
>> go away and I see what I'd expect to see.  As far as I can tell, I'm using
>> the "en-US.UTF-8" locale, though -- not the "C" one.  But maybe I'm just
>> getting lucky because the locale encoding is UTF-8 and not, say, Shift-JIS
>> or something?  I dunno.
> 
> I confirmed that the code with .decode() work well in ja_JP.UTF-8 locale
> on Python 3.6 and Python 3.7 (but I got an error like "'ascii' codec can't
> decode byte 0xe3 in position 18: ordinal not in range(128)").                                                             ^ on Python 2.7
 
> With non UTF-8, non ascii locale, .decode() without specifying encoding
> causes UnicodeDecodeError on 'utf-8' codecs on Python 3.6 and 3.7.
> So if we want to allow this script run on non UTF-8, non ascii locale
> with Python 3, it needs additional code to set encoding.

I wrote "additional code".

[[[
Index: tools/backup/hot-backup.py.in
===================================================================
--- tools/backup/hot-backup.py.in	(revision 1878988)
+++ tools/backup/hot-backup.py.in	(working copy)
@@ -36,6 +36,7 @@
 ######################################################################
 
 import sys, os, getopt, stat, re, time, shutil, subprocess
+import locale
 import functools
 
 ######################################################################
@@ -204,10 +205,18 @@
   """Examine the repository REPO_DIR using the svnlook binary
   specified by SVNLOOK, and return the youngest revision."""
 
-  p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
-                       stdin=subprocess.PIPE,
-                       stdout=subprocess.PIPE,
-                       stderr=subprocess.PIPE)
+  if b'' == '':
+    p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
+                         stdin=subprocess.PIPE,
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE)
+  else:
+    p = subprocess.Popen([svnlook, 'youngest', '--', repo_dir],
+                         stdin=subprocess.PIPE,
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE,
+                         errors='backslashreplace', # foolproof
+                         encoding=locale.getpreferredencoding())
   infile, outfile, errfile = p.stdin, p.stdout, p.stderr
 
   stdout_lines = outfile.readlines()
@@ -220,7 +229,7 @@
     raise Exception("Unable to find the youngest revision for repository '%s'"
                     ": %s" % (repo_dir, stderr_lines[0].rstrip()))
 
-  return stdout_lines[0].strip().decode()
+  return stdout_lines[0].strip()
 
 ######################################################################
 # Main
]]]

I confirmed it works on Python 2.7, Python 3.7 on FreeBSD with
ja_JP.eucJP, ja_JP.UTF-8 locale. However I'm not sure if it works or not
on Windows, especially non English environment. It depends on that
the encoding of the error message of SVNLOOK equals to the result of 
locale.getreferredencoding() here.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>

Re: Python3 support for hot-backup.py

Posted by Yasuhito FUTATSUKI <fu...@bsdclub.org>.
On 2020/06/16 0:18, C. Michael Pilato wrote:
> On Mon, Jun 15, 2020 at 10:32 AM Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
> wrote:
> 
>> On 2020/06/15 21:38, C. Michael Pilato wrote:
>> Is it needed something like this?
>>
>> [[[
>> Index: tools/backup/hot-backup.py.in
>> ===================================================================
>> --- tools/backup/hot-backup.py.in       (revision 1878855)
>> +++ tools/backup/hot-backup.py.in       (working copy)
>> @@ -218,7 +218,7 @@
>>
>>    if stderr_lines:
>>      raise Exception("Unable to find the youngest revision for repository
>> '%s'"
>> -                    ": %s" % (repo_dir, stderr_lines[0].rstrip()))
>> +                    ": %s" % (repo_dir,
>> stderr_lines[0].rstrip().decode()))
>>
>>    return stdout_lines[0].strip().decode();
>>
>> ]]]
>>
>> If svnlook runs on locale other than C, this can cause UnicodeDecodeError,
>> but without applying it, the output from svnlook is embeded as a
>> representaion
>> of bytes object to the exception message, like b'...'.
>> (Although I think this script assuming C locale implicitly.)
>>
>> So I'm not sure which is better applying this or not.
>>
>> 'return stdout_lines[0].strip().decode();' is okey (except an extra ';',
>> but it is not critical), because stdout_lines[0] is always ascii string
>> in this context.
>>
> 
> I removed a couple of stray semicolons in  r1878859 -- thanks for catching
> that.
> 
> As for your question:  if I force "svnlook" to create errors (by setting
> the svnlook variable to "/usr/bin/svn"), today I see an error message with
> the b'...' formatting.  Adding .decode() as you suggested makes the b'...'
> go away and I see what I'd expect to see.  As far as I can tell, I'm using
> the "en-US.UTF-8" locale, though -- not the "C" one.  But maybe I'm just
> getting lucky because the locale encoding is UTF-8 and not, say, Shift-JIS
> or something?  I dunno.

I confirmed that the code with .decode() work well in ja_JP.UTF-8 locale
on Python 3.6 and Python 3.7 (but I got an error like "'ascii' codec can't
decode byte 0xe3 in position 18: ordinal not in range(128)").

With non UTF-8, non ascii locale, .decode() without specifying encoding
causes UnicodeDecodeError on 'utf-8' codecs on Python 3.6 and 3.7.

Without .decode(), Python 3.6:
[[[
$ env LC_MESSAGES=ja_JP.eucJP LC_CTYPE=ja_JP.eucJP /usr/local/bin/python3.6m tools/backup/hot-backup.py /home/futatuki/tmp/t /home/futatuki/tmp/svn-test/tt
Beginning hot backup of '/home/futatuki/tmp/t'.
Unable to find the youngest revision for repository '/home/futatuki/tmp/t': b"svnlook: E000002: \xa5\xd5\xa5\xa1\xa5\xa4\xa5\xeb '/home/futatuki/tmp/t/format' \xa4\xf2\xb3\xab\xa4\xb1\xa4\xde\xa4\xbb\xa4\xf3: \xa4\xbd\xa4\xce\xa4\xe8\xa4\xa6\xa4\xca\xa5\xd5\xa5\xa1\xa5\xa4\xa5\xeb\xa4\xde\xa4\xbf\xa4\xcf\xa5\xc7\xa5\xa3\xa5\xec\xa5\xaf\xa5\xc8\xa5\xea\xa4\xcf\xa4\xa2\xa4\xea\xa4\xde\xa4\xbb\xa4\xf3"
]]]

With .decode(), Python 3.6:
[[[
$ env LC_MESSAGES=ja_JP.eucJP LC_CTYPE=ja_JP.eucJP /usr/local/bin/python3.6m tools/backup/hot-backup.py /home/futatuki/tmp/t /home/futatuki/tmp/svn-test/tt
Beginning hot backup of '/home/futatuki/tmp/t'.
'utf-8' codec can't decode byte 0xa5 in position 18: invalid start byte
]]]

cf. With .decode(), UTF-8 locale (Japanese):
(This is also what we want to see on ja_JP.eucJP terminal with ja_JP.eucJP locale.)
[[[
$ env LC_MESSAGES=ja_JP.UTF-8 LC_CTYPE=ja_JP.UTF-8 /usr/local/bin/python3.6m tools/backup/hot-backup.py /home/futatuki/tmp/t /home/futatuki/tmp/svn-test/tt
Beginning hot backup of '/home/futatuki/tmp/t'.
Unable to find the youngest revision for repository '/home/futatuki/tmp/t': svnlook: E000002: ファイル '/home/futatuki/tmp/t/format' を開けません: そのようなファイルまたはディレクトリはありません
]]]

So if we want to allow this script run on non UTF-8, non ascii locale
with Python 3, it needs additional code to set encoding.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>

Re: Python3 support for hot-backup.py

Posted by "C. Michael Pilato" <cm...@red-bean.com>.
On Mon, Jun 15, 2020 at 10:32 AM Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
wrote:

> On 2020/06/15 21:38, C. Michael Pilato wrote:
> Is it needed something like this?
>
> [[[
> Index: tools/backup/hot-backup.py.in
> ===================================================================
> --- tools/backup/hot-backup.py.in       (revision 1878855)
> +++ tools/backup/hot-backup.py.in       (working copy)
> @@ -218,7 +218,7 @@
>
>    if stderr_lines:
>      raise Exception("Unable to find the youngest revision for repository
> '%s'"
> -                    ": %s" % (repo_dir, stderr_lines[0].rstrip()))
> +                    ": %s" % (repo_dir,
> stderr_lines[0].rstrip().decode()))
>
>    return stdout_lines[0].strip().decode();
>
> ]]]
>
> If svnlook runs on locale other than C, this can cause UnicodeDecodeError,
> but without applying it, the output from svnlook is embeded as a
> representaion
> of bytes object to the exception message, like b'...'.
> (Although I think this script assuming C locale implicitly.)
>
> So I'm not sure which is better applying this or not.
>
> 'return stdout_lines[0].strip().decode();' is okey (except an extra ';',
> but it is not critical), because stdout_lines[0] is always ascii string
> in this context.
>

I removed a couple of stray semicolons in  r1878859 -- thanks for catching
that.

As for your question:  if I force "svnlook" to create errors (by setting
the svnlook variable to "/usr/bin/svn"), today I see an error message with
the b'...' formatting.  Adding .decode() as you suggested makes the b'...'
go away and I see what I'd expect to see.  As far as I can tell, I'm using
the "en-US.UTF-8" locale, though -- not the "C" one.  But maybe I'm just
getting lucky because the locale encoding is UTF-8 and not, say, Shift-JIS
or something?  I dunno.

Re: Python3 support for hot-backup.py

Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2020/06/15 21:38, C. Michael Pilato wrote:
> Reviewed, tested, and compared with similar Python3 conversions to other
> scripts.  All looks good, so committed it:
> 
> Sending        backup/hot-backup.py.in
> Transmitting file data .done
> Committing transaction...
> Committed revision 1878855.

Is it needed something like this?

[[[
Index: tools/backup/hot-backup.py.in
===================================================================
--- tools/backup/hot-backup.py.in       (revision 1878855)
+++ tools/backup/hot-backup.py.in       (working copy)
@@ -218,7 +218,7 @@
 
   if stderr_lines:
     raise Exception("Unable to find the youngest revision for repository '%s'"
-                    ": %s" % (repo_dir, stderr_lines[0].rstrip()))
+                    ": %s" % (repo_dir, stderr_lines[0].rstrip().decode()))
 
   return stdout_lines[0].strip().decode();
 
]]]

If svnlook runs on locale other than C, this can cause UnicodeDecodeError,
but without applying it, the output from svnlook is embeded as a representaion
of bytes object to the exception message, like b'...'.
(Although I think this script assuming C locale implicitly.)

So I'm not sure which is better applying this or not.

'return stdout_lines[0].strip().decode();' is okey (except an extra ';', 
but it is not critical), because stdout_lines[0] is always ascii string
in this context.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>

Re: Python3 support for hot-backup.py

Posted by "C. Michael Pilato" <cm...@red-bean.com>.
Reviewed, tested, and compared with similar Python3 conversions to other
scripts.  All looks good, so committed it:

Sending        backup/hot-backup.py.in
Transmitting file data .done
Committing transaction...
Committed revision 1878855.

On Mon, Jun 15, 2020 at 4:52 AM Petyovský Petr <pe...@feec.vutbr.cz>
wrote:

> Hi there.
>
> Is there someone who is reponsible for the hot-backup.py script, that
> here answer me?
>
> Or I can create feature request on the subversion Apache Jira directly
> without discussion here?
>
> Thanks for reactions.
>
> Regards,
> Peter
>
>
> Dne 11.6.2020 v 13:42 Petyovský Petr napsal(a):
> > Hi developers.
> >
> > I upgraded my server to the fedora core 32 which completely drop Python2.
> >
> > So I prepare some quick and dirty patch (I really dont know Python at
> > all) which makes hot-backup.py script working again under Python3
> > environment.
> >
> > Maybe some real python programmer is able improve my patch and include
> > all fixes to the trunk.
> >
> > Regards,
> > Peter
>
>
> --
> Ing. Petr Petyovsky, Ph.D.
>
> Brno University of Technology
> Faculty of Electrical Engineering and Communication
> Department of Control and Instrumentation
> Technicka 3082/12, 616 00 Brno
> Czech Republic
> European Union
>

Re: Python3 support for hot-backup.py

Posted by Petyovský Petr <pe...@feec.vutbr.cz>.
Hi there.

Is there someone who is reponsible for the hot-backup.py script, that 
here answer me?

Or I can create feature request on the subversion Apache Jira directly 
without discussion here?

Thanks for reactions.

Regards,
Peter


Dne 11.6.2020 v 13:42 Petyovský Petr napsal(a):
> Hi developers.
> 
> I upgraded my server to the fedora core 32 which completely drop Python2.
> 
> So I prepare some quick and dirty patch (I really dont know Python at 
> all) which makes hot-backup.py script working again under Python3 
> environment.
> 
> Maybe some real python programmer is able improve my patch and include
> all fixes to the trunk.
> 
> Regards,
> Peter


-- 
Ing. Petr Petyovsky, Ph.D.

Brno University of Technology
Faculty of Electrical Engineering and Communication
Department of Control and Instrumentation
Technicka 3082/12, 616 00 Brno
Czech Republic
European Union