Supported SWIG version on swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-21 Thread Yasuhito FUTATSUKI

On 2019/10/21 19:55, Jun Omae wrote:

Hi,

I'm trying to build and test swig-py3 branch (r1868677) on Ubuntu 16.04 with
Python 3.7, however I get FAILED(failures=16, errors=22) from the unit tests.

Investigating the issue with helps of Yasuhito, that is caused by using old
SWIG version with no SWIG_PYTHON_STRICT_BYTE_CHAR feature. The
SWIG_PYTHON_STRICT_BYTE_CHAR feature is available since SWIG 3.0.9 but SWIG is
3.0.8 in Ubuntu 16.04.

I consider that we should warn the required SWIG version at least.


[1] https://github.com/swig/swig/blob/rel-3.0.10/CHANGES#L160


Thank you for your report. I think if that feature or some other changes on
swig-py3 breaks Python 2 (or accidentally Ruby and/or Perl bindings),
we should bump required SWIG verersion or resolve issues with older SWIGs.
However if it affect only with Python 3, we only need to restrict
per Language bindings requirement.

Anyway, I think we need more test with older SWIG (or restrict required
SWIG version if we can't test).

Thanks,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-21 Thread Jun Omae

Hi,

I'm trying to build and test swig-py3 branch (r1868677) on Ubuntu 16.04 with
Python 3.7, however I get FAILED(failures=16, errors=22) from the unit tests.

Investigating the issue with helps of Yasuhito, that is caused by using old
SWIG version with no SWIG_PYTHON_STRICT_BYTE_CHAR feature. The
SWIG_PYTHON_STRICT_BYTE_CHAR feature is available since SWIG 3.0.9 but SWIG is
3.0.8 in Ubuntu 16.04.

I consider that we should warn the required SWIG version at least.


[1] https://github.com/swig/swig/blob/rel-3.0.10/CHANGES#L160


[[[
Index: build/ac-macros/swig.m4
===
--- build/ac-macros/swig.m4 (revision 1868677)
+++ build/ac-macros/swig.m4 (working copy)
@@ -91,12 +91,12 @@
 AC_MSG_RESULT([$SWIG_VERSION_RAW])
 # If you change the required swig version number, don't forget to update:
 #   subversion/bindings/swig/INSTALL
-if test -n "$SWIG_VERSION" && test "$SWIG_VERSION" -ge "103024"; then
+if test -n "$SWIG_VERSION" && test "$SWIG_VERSION" -ge "39"; then
   SWIG_SUITABLE=yes
 else
   SWIG_SUITABLE=no
   AC_MSG_WARN([Detected SWIG version $SWIG_VERSION_RAW])
-  AC_MSG_WARN([Subversion requires SWIG >= 1.3.24])
+  AC_MSG_WARN([Subversion requires SWIG >= 3.0.9])
 fi
   fi

Index: subversion/bindings/swig/INSTALL
===
--- subversion/bindings/swig/INSTALL(revision 1868677)
+++ subversion/bindings/swig/INSTALL(working copy)
@@ -65,7 +65,7 @@


 Step 1:  Install a suitable version of SWIG (which is
- currently SWIG version 1.3.24 or later).
+ currently SWIG version 3.0.9 or later).

 * Perhaps your distribution packages a suitable version - if it does
   install it, and skip to the last bullet point in this section.
@@ -72,7 +72,7 @@

 * Go to http://www.swig.org/, download the source tarball, and unpack.

-* In the SWIG-1.3.xx directory, run ./configure.
+* In the SWIG-3.0.xx directory, run ./configure.

 If you plan to build the Python bindings, and have a system
 with more than one version of Python installed, you may need
@@ -95,7 +95,7 @@
 Run 'make && make install'

 * To verify you have SWIG installed correctly, run "swig -version"
-  from the command line. SWIG should report that it is version 1.3.24
+  from the command line. SWIG should report that it is version 3.0.9
   or newer.

 Step 1a: Install py3c library if building Python SWIG bindings.
]]]


--
Jun Omae  (大前 潤)


Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-20 Thread Johan Corveleyn
Op zo 20 okt. 2019 03:51 schreef Nathan Hartman :

> On Sat, Oct 19, 2019 at 2:11 PM Branko Čibej  wrote:
> > By the principle of least surprise, I think it
> > would be better to merge to trunk, create a
> > new 1.13.0 release candidate
>
> +1
>
> > and (maybe?) restart the soak.
>
> I support this idea even if the soak must restart or be extended.
>

+1. Since 1.13 contains so very little, I think it's good to be a bit
flexible with our planned timing here, to get this on board. I.e. let's
merge it in, cut a new rc, and restart the soak.

-- 
Johan


Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-20 Thread Branko Čibej
On 20.10.2019 04:05, Daniel Shahaf wrote:
> Nathan Hartman wrote on Sun, 20 Oct 2019 01:49 +00:00:
>> I support this idea even if the soak must restart or be extended.
> Brainstorming: How about letting the soak continue but documenting in
> the release notes, release announcements, etc., that we'll be adding swig-
> py3 support in a patch release?  This way, swig-py2 users will know not
> to upgrade from 1.12.x until after the destabilizing changes are made.

The thing is that the swig-py3 branch introduces a new dependency (py3c)
for Py2 bindings, not just for Py3. That's a big deal even for
packagers, IMO.

(Which reminds me: get-deps.sh still needs to be updated to download py3c.)

> I assume the parts of the swig-py3 branch outside subversion/bindings/
> aren't destabilizing and would not be a concern to backport in a patch
> release.

Those are mainly test fixes, so sure.

-- Brane


Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-19 Thread Daniel Shahaf
Nathan Hartman wrote on Sun, 20 Oct 2019 01:49 +00:00:
> I support this idea even if the soak must restart or be extended.

Brainstorming: How about letting the soak continue but documenting in
the release notes, release announcements, etc., that we'll be adding swig-
py3 support in a patch release?  This way, swig-py2 users will know not
to upgrade from 1.12.x until after the destabilizing changes are made.

I assume the parts of the swig-py3 branch outside subversion/bindings/
aren't destabilizing and would not be a concern to backport in a patch
release.

Cheers,

Daniel


Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-19 Thread Nathan Hartman
On Sat, Oct 19, 2019 at 2:11 PM Branko Čibej  wrote:
> By the principle of least surprise, I think it
> would be better to merge to trunk, create a
> new 1.13.0 release candidate

+1

> and (maybe?) restart the soak.

I support this idea even if the soak must restart or be extended.

Rationale:

* we still get 1.13 out in time for the holidays

* we get one STS release with Py3 before the
  next LTS release

* we support Py3 before Py2 goes "out of
  support" -- there's no "lapse in coverage" for
  customers who have IT policies about that

(One corporate or government customer wrote
to users@ some months ago and specifically
mentioned concern because of such a policy;
I'd love to reply and say that Py3 is supported
on time.)

Bottom line: we'll be a little late in the release but in exchange we'll
have one really whiz-bang compelling new feature -- that people have been
asking for -- to write about in the
release notes.

Nathan


Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-19 Thread Branko Čibej
On 19.10.2019 12:06, Johan Corveleyn wrote:
> On Sat, Oct 19, 2019 at 1:23 AM Daniel Shahaf  wrote:
>> Yasuhito FUTATSUKI wrote on Fri, Oct 18, 2019 at 23:09:19 +0900:
>>> On 2019/10/16 21:12, Johan Corveleyn wrote:
 This makes me wonder: should that be fixed specifically on trunk, and
 nominated for backport to 1.13, so we can possibly claim basic support
 for Python 3 in our build and test processes (in at least one released
 version) before the end of this year?

 Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
 for backport to 1.13, so we can have Python 3 support, including swig
 bindings?
>>> I prefer the latter, as one of users :) I want to use
>>> tools/hook-scripts/mailer/mailer.py with Python 3.
>> If we want this to happen, we should first of all complete the swig-py3 
>> branch
>> and merge it to trunk.
> Yes, and I think the branch is now ready for merge to trunk.
>
>> What's not clear to me is what would happen afterwards.  Is anyone proposing 
>> to
>> delay 1.13.0 until swig-py3 is merged (remember that we are already more than
>> halfway through the soak)?  If not, how would merging swig-py3 to 1.13.x
>> coexist with the premise of "no destabilizing changes in patch releases"?
>> Would we have to delay merging swig-py3 to 1.13.x until January, when py2 has
>> finally gone out of support?
> I wouldn't postpone 1.13.0 for it. I suppose the best way is that we
> propose it for backport for 1.13.1, shortly after 1.13.0 has been
> released.
> Also: the swig-py3 branch does not break our support for py2. With
> that branch, both py2 and py3 swig-bindings can be built and run fine.

But it does introduce major changes in the bindings, for Py2 as well.
I'd really, really rather not drop them on unsuspecting users in a patch
release.

By the principle of least surprise, I think it would be better to merge
to trunk, create a new 1.13.0 release candidate and (maybe?) restart the
soak.


>> What should we do about swig-py3 support in 1.12.x and 1.10.x, which are 
>> both LTS?
> Only 1.10.x is LTS (and 1.9.x perhaps? But ISTR we eol'ed that). I
> suppose a backport to 1.10.x would be a good idea, so we have at least
> one LTS release this year that supports py3.


-0, for the same reason as above. We'll have the LTS that supports
Python 3 in a few months. Anyone who cares so much about Python 3
support will be able to use 1.13.0 (or .x). And of course ... EOL or
not, Python 2 will be around for a while yet.

>> Should we say anything about swig-py2 / swig-py3 in the release notes 
>> _today_,
>> before 1.13.0 has been released, about our plans for 1.13.x patch releases?


Given that I prefer releasing this with 1.13.0, the answer is, of
course, "yes."

-- Brane



Re: 1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-19 Thread Johan Corveleyn
On Sat, Oct 19, 2019 at 1:23 AM Daniel Shahaf  wrote:
>
> Yasuhito FUTATSUKI wrote on Fri, Oct 18, 2019 at 23:09:19 +0900:
> > On 2019/10/16 21:12, Johan Corveleyn wrote:
> > > This makes me wonder: should that be fixed specifically on trunk, and
> > > nominated for backport to 1.13, so we can possibly claim basic support
> > > for Python 3 in our build and test processes (in at least one released
> > > version) before the end of this year?
> > >
> > > Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
> > > for backport to 1.13, so we can have Python 3 support, including swig
> > > bindings?
> >
> > I prefer the latter, as one of users :) I want to use
> > tools/hook-scripts/mailer/mailer.py with Python 3.
>
> If we want this to happen, we should first of all complete the swig-py3 branch
> and merge it to trunk.

Yes, and I think the branch is now ready for merge to trunk.

> What's not clear to me is what would happen afterwards.  Is anyone proposing 
> to
> delay 1.13.0 until swig-py3 is merged (remember that we are already more than
> halfway through the soak)?  If not, how would merging swig-py3 to 1.13.x
> coexist with the premise of "no destabilizing changes in patch releases"?
> Would we have to delay merging swig-py3 to 1.13.x until January, when py2 has
> finally gone out of support?

I wouldn't postpone 1.13.0 for it. I suppose the best way is that we
propose it for backport for 1.13.1, shortly after 1.13.0 has been
released.
Also: the swig-py3 branch does not break our support for py2. With
that branch, both py2 and py3 swig-bindings can be built and run fine.

> What should we do about swig-py3 support in 1.12.x and 1.10.x, which are both 
> LTS?

Only 1.10.x is LTS (and 1.9.x perhaps? But ISTR we eol'ed that). I
suppose a backport to 1.10.x would be a good idea, so we have at least
one LTS release this year that supports py3.

> Should we say anything about swig-py2 / swig-py3 in the release notes _today_,
> before 1.13.0 has been released, about our plans for 1.13.x patch releases?

I think we should mention it at least in the 1.13.0 release notes, in
the known issues section, and announce our plan to address it in
1.13.1 or something like that?

-- 
Johan


1.13.x and swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

2019-10-18 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Fri, Oct 18, 2019 at 23:09:19 +0900:
> On 2019/10/16 21:12, Johan Corveleyn wrote:
> > This makes me wonder: should that be fixed specifically on trunk, and
> > nominated for backport to 1.13, so we can possibly claim basic support
> > for Python 3 in our build and test processes (in at least one released
> > version) before the end of this year?
> > 
> > Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
> > for backport to 1.13, so we can have Python 3 support, including swig
> > bindings?
> 
> I prefer the latter, as one of users :) I want to use
> tools/hook-scripts/mailer/mailer.py with Python 3.

If we want this to happen, we should first of all complete the swig-py3 branch
and merge it to trunk.

What's not clear to me is what would happen afterwards.  Is anyone proposing to
delay 1.13.0 until swig-py3 is merged (remember that we are already more than
halfway through the soak)?  If not, how would merging swig-py3 to 1.13.x
coexist with the premise of "no destabilizing changes in patch releases"?
Would we have to delay merging swig-py3 to 1.13.x until January, when py2 has
finally gone out of support?

What should we do about swig-py3 support in 1.12.x and 1.10.x, which are both 
LTS?

Should we say anything about swig-py2 / swig-py3 in the release notes _today_,
before 1.13.0 has been released, about our plans for 1.13.x patch releases?


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-18 Thread Yasuhito FUTATSUKI

On 2019/10/16 21:12, Johan Corveleyn wrote:

On Wed, Oct 16, 2019 at 1:37 PM Yasuhito FUTATSUKI  wrote:


On 2019/10/16 18:10, Johan Corveleyn wrote:

Python 3 for the build and test process is only supported on *nix, not

on Windows.

[[[
Traceback (most recent call last):
File "win-tests.py", line 134, in 
  cp.items('options'))
File "build\generator\gen_win_dependencies.py", line 306, in __init__
  self.find_libraries(False)
File "build\generator\gen_win_dependencies.py", line 327, in find_libraries
  self._find_jdk(show_warnings)
File "build\generator\gen_win_dependencies.py", line 1085, in _find_jdk
  vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
File "C:\Python37\lib\re.py", line 183, in search
  return _compile(pattern, flags).search(string)
TypeError: cannot use a string pattern on a bytes-like object
]]]

The fix is probably easy, but I'm just noting it here so we don't get
ahead of ourselves.



It seems the change addressing for it on swig-py3 branch is a part of
r1822485, the hunks attached.


Ack.


I've looked over the log on swig-py3 branch and picked up more hunks,
and I've made a patch against trunk@1868582.

Could you please test on trunk on Windows with this patch?


This makes me wonder: should that be fixed specifically on trunk, and
nominated for backport to 1.13, so we can possibly claim basic support
for Python 3 in our build and test processes (in at least one released
version) before the end of this year?

Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
for backport to 1.13, so we can have Python 3 support, including swig
bindings?


I prefer the latter, as one of users :) I want to use
tools/hook-scripts/mailer/mailer.py with Python 3.

Thanks,
--
Yasuhito FUTATSUKI 
Fix issue on test on Windows with Python 3.

(cherry-pick of hunks from r1822485 and its fix r1822486, on branch swig-py3) 

* build/generator/gen_base.py
  (IncludeDependencyInfo._scan_for_includes): Ensure file data is
   properly encoded as UTF8. (r1822485, r1822486)

* build/generator/gen_win.py
  (WinGeneratorBase.makeguid): Ensure data input to hashlib is binary
   and not Unicode. (r1822485)

* build/generator/gen_win_dependencies.py
  (GenDependenciesBase._find_swig,
   GenDependenciesBase._find_jdk): Ensure output from subprocess is
   properly decoded. (r1822485)

Index: build/generator/gen_base.py
===
--- build/generator/gen_base.py (revision 1868582)
+++ build/generator/gen_base.py (working copy)
@@ -1273,7 +1273,8 @@
 Return a dictionary with included full file names as keys and None as
 values."""
 hdrs = { }
-for line in fileinput.input(fname):
+
+for line in fileinput.FileInput(fname, 
openhook=fileinput.hook_encoded("utf-8")):
   match = self._re_include.match(line)
   if not match:
 continue
Index: build/generator/gen_win.py
===
--- build/generator/gen_win.py  (revision 1868582)
+++ build/generator/gen_win.py  (working copy)
@@ -158,6 +158,13 @@
 ### implement this from scratch using the algorithms described in
 ### http://www.webdav.org/specs/draft-leach-uuids-guids-01.txt
 
+# Ensure data is in byte representation.  If it doesn't have an encode
+# attribute, assume it is already in the correct form.
+try:
+  data = data.encode('utf8')
+except AttributeError:
+  pass
+
 myhash = hashlib_md5(data).hexdigest()
 
 guid = ("{%s-%s-%s-%s-%s}" % (myhash[0:8], myhash[8:12],
Index: build/generator/gen_win_dependencies.py
===
--- build/generator/gen_win_dependencies.py (revision 1868582)
+++ build/generator/gen_win_dependencies.py (working copy)
@@ -1080,7 +1080,7 @@
   outfp = subprocess.Popen([os.path.join(jdk_path, 'bin', 'javac.exe'),
'-version'], stdout=subprocess.PIPE,
stderr=subprocess.STDOUT).stdout
-  line = outfp.read()
+  line = outfp.read().decode('utf8')
   if line:
 vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
   else:
@@ -1138,7 +1138,7 @@
 try:
   fp = subprocess.Popen([self.swig_exe, '-version'],
 stdout=subprocess.PIPE).stdout
-  txt = fp.read()
+  txt = fp.read().decode('utf8')
   if txt:
 vermatch = re.search(r'^SWIG\ Version\ (\d+)\.(\d+)\.(\d+)', txt, re.M)
   else:
@@ -1166,7 +1166,7 @@
 try:
   fp = subprocess.Popen([self.swig_exe, '-swiglib'],
 stdout=subprocess.PIPE).stdout
-  lib_dir = fp.readline().strip()
+  lib_dir = fp.readline().decode('utf8').strip()
   fp.close()
 except OSError:
   lib_dir = None


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-17 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Thu, 17 Oct 2019 22:57 +00:00:
> I'm sorry, I missed to answer to these questions.
> 
> On 2019/10/16 16:55, Daniel Shahaf wrote:
> > Good catch.  Yes, we should update INSTALL to reflect that Python 3 is
> > supported for the build and test process, even if it's not yet supported
> > by the swig bindings in trunk and 1.13.x.  Would you be able to update
> > that?  You're welcome to just commit changes to trunk/INSTALL directly,
> > if that's easier for you.
> 
> Yes, I'll do it, after resolved issues on test with Python3, the false
> positive issue and the test on Windows issue.

Thank you.

> > As to the test suite patches, I think they're ready to be committed,
> > aren't they?
> 
> Yes, at least I also think so, putting aside commit log message.

In this case, please do go ahead and commit them :-)

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-17 Thread Yasuhito FUTATSUKI

I'm sorry, I missed to answer to these questions.

On 2019/10/16 16:55, Daniel Shahaf wrote:

Good catch.  Yes, we should update INSTALL to reflect that Python 3 is
supported for the build and test process, even if it's not yet supported
by the swig bindings in trunk and 1.13.x.  Would you be able to update
that?  You're welcome to just commit changes to trunk/INSTALL directly,
if that's easier for you.


Yes, I'll do it, after resolved issues on test with Python3, the false
positive issue and the test on Windows issue.


As to the test suite patches, I think they're ready to be committed,
aren't they?


Yes, at least I also think so, putting aside commit log message.

Cheers,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-16 Thread Daniel Shahaf
Johan Corveleyn wrote on Wed, 16 Oct 2019 12:12 +00:00:
> This makes me wonder: should that be fixed specifically on trunk, and
> nominated for backport to 1.13, so we can possibly claim basic support
> for Python 3 in our build and test processes (in at least one released
> version) before the end of this year?

+1

> Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
> for backport to 1.13, so we can have Python 3 support, including swig
> bindings?

Good question.

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-16 Thread Johan Corveleyn
On Wed, Oct 16, 2019 at 1:37 PM Yasuhito FUTATSUKI  wrote:
>
> On 2019/10/16 18:10, Johan Corveleyn wrote:
> > > Python 3 for the build and test process is only supported on *nix, not
> > on Windows.
> >
> > [[[
> > Traceback (most recent call last):
> >File "win-tests.py", line 134, in 
> >  cp.items('options'))
> >File "build\generator\gen_win_dependencies.py", line 306, in __init__
> >  self.find_libraries(False)
> >File "build\generator\gen_win_dependencies.py", line 327, in 
> > find_libraries
> >  self._find_jdk(show_warnings)
> >File "build\generator\gen_win_dependencies.py", line 1085, in _find_jdk
> >  vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
> >File "C:\Python37\lib\re.py", line 183, in search
> >  return _compile(pattern, flags).search(string)
> > TypeError: cannot use a string pattern on a bytes-like object
> > ]]]
> >
> > The fix is probably easy, but I'm just noting it here so we don't get
> > ahead of ourselves.
> >
>
> It seems the change addressing for it on swig-py3 branch is a part of
> r1822485, the hunks attached.

Ack.

This makes me wonder: should that be fixed specifically on trunk, and
nominated for backport to 1.13, so we can possibly claim basic support
for Python 3 in our build and test processes (in at least one released
version) before the end of this year?

Or should we reintegrate the swig-py3 branch ASAP, and nominate *that*
for backport to 1.13, so we can have Python 3 support, including swig
bindings?

-- 
Johan


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-16 Thread Yasuhito FUTATSUKI

On 2019/10/16 18:10, Johan Corveleyn wrote:

> Python 3 for the build and test process is only supported on *nix, not
on Windows.

[[[
Traceback (most recent call last):
   File "win-tests.py", line 134, in 
 cp.items('options'))
   File "build\generator\gen_win_dependencies.py", line 306, in __init__
 self.find_libraries(False)
   File "build\generator\gen_win_dependencies.py", line 327, in find_libraries
 self._find_jdk(show_warnings)
   File "build\generator\gen_win_dependencies.py", line 1085, in _find_jdk
 vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
   File "C:\Python37\lib\re.py", line 183, in search
 return _compile(pattern, flags).search(string)
TypeError: cannot use a string pattern on a bytes-like object
]]]

The fix is probably easy, but I'm just noting it here so we don't get
ahead of ourselves.



It seems the change addressing for it on swig-py3 branch is a part of
r1822485, the hunks attached.
--
Yasuhito FUTATSUKI 
Index: gen_win_dependencies.py
===
--- gen_win_dependencies.py (revision 1822484)
+++ gen_win_dependencies.py (revision 1822485)
@@ -1069,7 +1122,7 @@
 try:
   outfp = subprocess.Popen([os.path.join(jdk_path, 'bin', 'javah.exe'),
'-version'], stdout=subprocess.PIPE).stdout
-  line = outfp.read()
+  line = outfp.read().decode('utf8')
   if line:
 vermatch = re.search(r'"(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)"', line, 
re.M)
   else:
@@ -1127,7 +1180,7 @@
 try:
   fp = subprocess.Popen([self.swig_exe, '-version'],
 stdout=subprocess.PIPE).stdout
-  txt = fp.read()
+  txt = fp.read().decode('utf8')
   if txt:
 vermatch = re.search(r'^SWIG\ Version\ (\d+)\.(\d+)\.(\d+)', txt, re.M)
   else:
@@ -1155,7 +1208,7 @@
 try:
   fp = subprocess.Popen([self.swig_exe, '-swiglib'],
 stdout=subprocess.PIPE).stdout
-  lib_dir = fp.readline().strip()
+  lib_dir = fp.readline().decode('utf8').strip()
   fp.close()
 except OSError:
   lib_dir = None


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-16 Thread Johan Corveleyn
On Wed, Oct 16, 2019 at 9:55 AM Daniel Shahaf  wrote:
>
> Yasuhito FUTATSUKI wrote on Wed, 16 Oct 2019 04:40 +00:00:
> > On 2019/10/16 10:36, Daniel Shahaf wrote:
> > > Yasuhito FUTATSUKI wrote on Tue, Oct 15, 2019 at 16:19:46 +0900:
> > >> On 2019-10-15 07:04, Daniel Shahaf wrote:
> > >>> Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
> >  On 2019/10/13 7:24, Daniel Shahaf wrote:
> >
> > >>> Also, what about the svnadmin_tests.py patch you posted upthread?  Is
> > >>> there a reason not to go ahead and commit it to trunk?  (and even 
> > >>> nominate
> > >>> it for backport in 1.13.x/STATUS)
> > >>
> > >> I think that is no probrem.
> > >
> > > Let me elaborate.  By my reckoning, «make check» on trunk has a false 
> > > positive
> > > when ${PYTHON} is python3; and furthermore, python2 will reach EOL during 
> > > the
> > > lifetime of the 1.13 branch; therefore, we should fix the false positive 
> > > and
> > > backport the fix to supported branches.  What's your view?
> >
> > I agree with you, because I think we should publish at least one relase
> > support Python 3, so that we can remove the sentence "Note that Python 3.x
> > is not supported and most likely won't work." in "Dependencies in Detail"
> > section of INSTALL file, before EOL of Python 2.
>
> Good catch.  Yes, we should update INSTALL to reflect that Python 3 is
> supported for the build and test process, even if it's not yet supported
> by the swig bindings in trunk and 1.13.x.  Would you be able to update
> that?  You're welcome to just commit changes to trunk/INSTALL directly,
> if that's easier for you.

Python 3 for the build and test process is only supported on *nix, not
on Windows.

[[[
Traceback (most recent call last):
  File "win-tests.py", line 134, in 
cp.items('options'))
  File "build\generator\gen_win_dependencies.py", line 306, in __init__
self.find_libraries(False)
  File "build\generator\gen_win_dependencies.py", line 327, in find_libraries
self._find_jdk(show_warnings)
  File "build\generator\gen_win_dependencies.py", line 1085, in _find_jdk
vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
  File "C:\Python37\lib\re.py", line 183, in search
return _compile(pattern, flags).search(string)
TypeError: cannot use a string pattern on a bytes-like object
]]]

The fix is probably easy, but I'm just noting it here so we don't get
ahead of ourselves.

-- 
Johan


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-16 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Wed, 16 Oct 2019 04:40 +00:00:
> On 2019/10/16 10:36, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Tue, Oct 15, 2019 at 16:19:46 +0900:
> >> On 2019-10-15 07:04, Daniel Shahaf wrote:
> >>> Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
>  On 2019/10/13 7:24, Daniel Shahaf wrote:
>   
> >>> Also, what about the svnadmin_tests.py patch you posted upthread?  Is
> >>> there a reason not to go ahead and commit it to trunk?  (and even nominate
> >>> it for backport in 1.13.x/STATUS)
> >>
> >> I think that is no probrem.
> > 
> > Let me elaborate.  By my reckoning, «make check» on trunk has a false 
> > positive
> > when ${PYTHON} is python3; and furthermore, python2 will reach EOL during 
> > the
> > lifetime of the 1.13 branch; therefore, we should fix the false positive and
> > backport the fix to supported branches.  What's your view?
> 
> I agree with you, because I think we should publish at least one relase
> support Python 3, so that we can remove the sentence "Note that Python 3.x
> is not supported and most likely won't work." in "Dependencies in Detail"
> section of INSTALL file, before EOL of Python 2.

Good catch.  Yes, we should update INSTALL to reflect that Python 3 is
supported for the build and test process, even if it's not yet supported
by the swig bindings in trunk and 1.13.x.  Would you be able to update
that?  You're welcome to just commit changes to trunk/INSTALL directly,
if that's easier for you.

As to the test suite patches, I think they're ready to be committed,
aren't they?

Thanks,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Yasuhito FUTATSUKI

On 2019/10/16 10:36, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Tue, Oct 15, 2019 at 16:19:46 +0900:

On 2019-10-15 07:04, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:

On 2019/10/13 7:24, Daniel Shahaf wrote:
 

Also, what about the svnadmin_tests.py patch you posted upthread?  Is
there a reason not to go ahead and commit it to trunk?  (and even nominate
it for backport in 1.13.x/STATUS)


I think that is no probrem.


Let me elaborate.  By my reckoning, «make check» on trunk has a false positive
when ${PYTHON} is python3; and furthermore, python2 will reach EOL during the
lifetime of the 1.13 branch; therefore, we should fix the false positive and
backport the fix to supported branches.  What's your view?


I agree with you, because I think we should publish at least one relase
support Python 3, so that we can remove the sentence "Note that Python 3.x
is not supported and most likely won't work." in "Dependencies in Detail"
section of INSTALL file, before EOL of Python 2.

Cheers,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Daniel Shahaf
Johan Corveleyn wrote on Wed, Oct 16, 2019 at 01:07:32 +0200:
> I did have some trouble testing it with Python 3.7 though:
> 
> - First, I had to try it on the swig-py3 branch, because on trunk I
> get this when trying to run any test with py 3.7:
⋮
> I guess that's one of the issues fixed by the swig-py3 branch.
> 
> - Then, on the swig-py3 branch, with py 3.7.4 I ran into this issue:
> https://bugs.python.org/issue37549 (os.dup() fails for standard
> streams on Windows 7)
> This fails for any *.py test, because of line 836 in build/run_tests.py:
⋮
> 
> - Upgraded to py 3.7.5, in which the above issue seems to be fixed.
> Now, *.py tests still don't work. I get no output at all:
⋮
> However, if I run it with --log-to-stdout, the tests do work (with a
> lot of output on stdout). […]

Drive-by comment here, but: since Python 2 reaches EOL during the lifetime of
the 1.13.x branch, I think it would be nice for us to make sure «make check»
passes on Windows under Python 3…

> Conclusion: I can confirm your patch works on Windows, for both Pyton
> 2.7.16 and 3.7.5 on the swig-py3 branch. As for the stdout
> redirection, I guess there might still be a problem ... perhaps the
> fix for https://bugs.python.org/issue37549 is not sufficient for
> Windows 7 ... dunno. Maybe someone can try this on Windows 10 and see
> if it makes a difference.

… although then again, Windows 7 reaches EOL rather soon too, so if the
problem manifests only under Windows 7, it might well not be worth fixing.

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Tue, Oct 15, 2019 at 16:19:46 +0900:
> On 2019-10-15 07:04, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
> > > On 2019/10/13 7:24, Daniel Shahaf wrote:
> 
> > > I see. Now I agree it would suffice here.
> > > > So, how about:
> > > > 
> > > > 1. Make the test use non-binary mode for changing and reading the
> > > >  file 'lambda'.
> > > > 
> > > > 2. Locally revert the C part of r1841731 and make sure the modified test
> > > >  still (correctly) fails.  (That revision both added the test and
> > > >  fixed the bug the test checks for.)
> > > 
> > > So it looks sufficient to me.
> > 
> > Cool.  Will you perchance have time to do this?  No worries if not.
> 
> Yes, I'll do it on FreeBSD on tonight or tomorrow night (in JST :)).

Thanks.

> However I think it is also need to test for each 1 and 2 on Windows,
> because r1841736 and r1841743 also were attempt to fix this test
> on Windows, with Python 2.

I trust your judgement on this.

> > Also, what about the svnadmin_tests.py patch you posted upthread?  Is
> > there a reason not to go ahead and commit it to trunk?  (and even nominate
> > it for backport in 1.13.x/STATUS)
> 
> I think that is no probrem.

Let me elaborate.  By my reckoning, «make check» on trunk has a false positive
when ${PYTHON} is python3; and furthermore, python2 will reach EOL during the
lifetime of the 1.13 branch; therefore, we should fix the false positive and
backport the fix to supported branches.  What's your view?

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Johan Corveleyn
On Tue, Oct 15, 2019 at 6:20 PM Yasuhito FUTATSUKI  wrote:
>
> On 2019-10-15 17:17, Johan Corveleyn wrote:
> > On Tue, Oct 15, 2019 at 9:26 AM Yasuhito FUTATSUKI  
> > wrote:
> >>
> >> On 2019-10-15 07:04, Daniel Shahaf wrote:
> >>> Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
>  On 2019/10/13 7:24, Daniel Shahaf wrote:
> >>
>  I see. Now I agree it would suffice here.
> 
> > So, how about:
> >
> > 1. Make the test use non-binary mode for changing and reading the
> >   file 'lambda'.
>
> > 2. Locally revert the C part of r1841731 and make sure the modified test
> >   still (correctly) fails.  (That revision both added the test and
> >   fixed the bug the test checks for.)
>
> I overlooked comments in this test. On step 2 the test will continue to loop
> as far as resource is available, or until signaled.
>
> And yes, after 'svn merge -r1841731:1841730 
> subversion/libsvn_client/conflicts.c',
> the test can't reach patched line. So no more test is needed on step 2, both
> on Unix/Linux and on Windows.
>
> 
>  So it looks sufficient to me.
> >>>
> >>> Cool.  Will you perchance have time to do this?  No worries if not.
> >>
> >> Yes, I'll do it on FreeBSD on tonight or tomorrow night (in JST :)).
>
> with the attached patch, both with Python 2.7.15 and Python 3.7.0 on FreeBSD,
> tree_conflict_tests passed.
>
> >> However I think it is also need to test for each 1 and 2 on Windows,
> >> because r1841736 and r1841743 also were attempt to fix this test
> >> on Windows, with Python 2.
> >
> > Feel free to let me know if I need to test something on Windows.
>
> Thank you. Could you please test the tree_conflict_tests with this patch,
> both with Python 2 and Python 3 on Windows?

Okay, I can confirm that tree_conflict_tests works with Python 2.7.16
(both with and without the patch) and with 3.7.5 (with the patch) on
Windows 7.

I did have some trouble testing it with Python 3.7 though:

- First, I had to try it on the swig-py3 branch, because on trunk I
get this when trying to run any test with py 3.7:
[[[
Traceback (most recent call last):
  File "win-tests.py", line 134, in 
cp.items('options'))
  File "build\generator\gen_win_dependencies.py", line 306, in __init__
self.find_libraries(False)
  File "build\generator\gen_win_dependencies.py", line 327, in find_libraries
self._find_jdk(show_warnings)
  File "build\generator\gen_win_dependencies.py", line 1085, in _find_jdk
vermatch = re.search(r'(([0-9]+(\.[0-9]+)+)(_[._0-9]+)?)', line, re.M)
  File "C:\Python37\lib\re.py", line 183, in search
return _compile(pattern, flags).search(string)
TypeError: cannot use a string pattern on a bytes-like object
]]]

I guess that's one of the issues fixed by the swig-py3 branch.

- Then, on the swig-py3 branch, with py 3.7.4 I ran into this issue:
https://bugs.python.org/issue37549 (os.dup() fails for standard
streams on Windows 7)
This fails for any *.py test, because of line 836 in build/run_tests.py:
old_stdout = os.dup(sys.stdout.fileno())
It errors out with:
   OSError: [WinError 87] The parameter is incorrect

- Upgraded to py 3.7.5, in which the above issue seems to be fixed.
Now, *.py tests still don't work. I get no output at all:
[[[
C:\research\svn\dev\swig-py3>python win-tests.py --release -t tree_conflict .
'ruby' is not recognized as an internal or external command,
operable program or batch file.
Testing Release configuration on local repository.
[1/1] tree_conflict_tests.py
C:\research\svn\dev\swig-py3>
]]]

tests.log only contains one line:
START: tree_conflict_tests.py

However, if I run it with --log-to-stdout, the tests do work (with a
lot of output on stdout). I.e. I get some fails without your
fix_tree_conflict_tests_patch.txt, and all tests successful if I apply
the patch.


Conclusion: I can confirm your patch works on Windows, for both Pyton
2.7.16 and 3.7.5 on the swig-py3 branch. As for the stdout
redirection, I guess there might still be a problem ... perhaps the
fix for https://bugs.python.org/issue37549 is not sufficient for
Windows 7 ... dunno. Maybe someone can try this on Windows 10 and see
if it makes a difference.

-- 
Johan


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Yasuhito FUTATSUKI

On 2019-10-15 17:17, Johan Corveleyn wrote:

On Tue, Oct 15, 2019 at 9:26 AM Yasuhito FUTATSUKI  wrote:


On 2019-10-15 07:04, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:

On 2019/10/13 7:24, Daniel Shahaf wrote:



I see. Now I agree it would suffice here.


So, how about:

1. Make the test use non-binary mode for changing and reading the
  file 'lambda'.



2. Locally revert the C part of r1841731 and make sure the modified test
  still (correctly) fails.  (That revision both added the test and
  fixed the bug the test checks for.)


I overlooked comments in this test. On step 2 the test will continue to loop
as far as resource is available, or until signaled.

And yes, after 'svn merge -r1841731:1841730 
subversion/libsvn_client/conflicts.c',
the test can't reach patched line. So no more test is needed on step 2, both
on Unix/Linux and on Windows.



So it looks sufficient to me.


Cool.  Will you perchance have time to do this?  No worries if not.


Yes, I'll do it on FreeBSD on tonight or tomorrow night (in JST :)).


with the attached patch, both with Python 2.7.15 and Python 3.7.0 on FreeBSD,
tree_conflict_tests passed.


However I think it is also need to test for each 1 and 2 on Windows,
because r1841736 and r1841743 also were attempt to fix this test
on Windows, with Python 2.


Feel free to let me know if I need to test something on Windows.


Thank you. Could you please test the tree_conflict_tests with this patch,
both with Python 2 and Python 3 on Windows?

Thanks,
--
Yasuhito FUTATSUKI 
Index: subversion/tests/cmdline/svnadmin_tests.py
===
--- subversion/tests/cmdline/svnadmin_tests.py  (revision 1868481)
+++ subversion/tests/cmdline/svnadmin_tests.py  (working copy)
@@ -3859,7 +3859,7 @@
  sbox.repo_url)
 
   dump_lines = svntest.actions.run_and_verify_dump(sbox.repo_dir)
-  assert propval + '\n' in dump_lines
+  assert propval.encode() + b'\n' in dump_lines
 
 def check_recover_prunes_rep_cache(sbox, enable_rep_sharing):
   """Check 'recover' prunes the rep-cache while enable-rep-sharing is
Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868481)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1518,7 +1518,7 @@
   sbox.simple_move('A/B', 'A/B2')
   sbox.simple_commit()
   sbox.simple_update()
-  main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more 
content.\n")
+  main.file_append(sbox.ospath("A/B2/lambda"), "This is more content.\n")
   sbox.simple_commit()
   sbox.simple_update()
 
@@ -1541,7 +1541,7 @@
   # If everything works as expected the resolver will recommended a
   # resolution option and 'svn' will resolve the conflict automatically.
   # Verify that 'A1/B/lambda' contains the merged content:
-  contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
+  contents = open(sbox.ospath('A1/B/lambda'), 'r').readlines()
   svntest.verify.compare_and_display_lines(
 "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
 [ "This is the file 'lambda'.\n", "This is more content.\n"], contents)


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Johan Corveleyn
On Tue, Oct 15, 2019 at 9:26 AM Yasuhito FUTATSUKI  wrote:
>
> On 2019-10-15 07:04, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
> >> On 2019/10/13 7:24, Daniel Shahaf wrote:
>
> >> I see. Now I agree it would suffice here.
> >>
> >>> So, how about:
> >>>
> >>> 1. Make the test use non-binary mode for changing and reading the
> >>>  file 'lambda'.
> >>>
> >>> 2. Locally revert the C part of r1841731 and make sure the modified test
> >>>  still (correctly) fails.  (That revision both added the test and
> >>>  fixed the bug the test checks for.)
> >>
> >> So it looks sufficient to me.
> >
> > Cool.  Will you perchance have time to do this?  No worries if not.
>
> Yes, I'll do it on FreeBSD on tonight or tomorrow night (in JST :)).
> However I think it is also need to test for each 1 and 2 on Windows,
> because r1841736 and r1841743 also were attempt to fix this test
> on Windows, with Python 2.

Feel free to let me know if I need to test something on Windows.

-- 
Johan


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-15 Thread Yasuhito FUTATSUKI

On 2019-10-15 07:04, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:

On 2019/10/13 7:24, Daniel Shahaf wrote:



I see. Now I agree it would suffice here.
   

So, how about:

1. Make the test use non-binary mode for changing and reading the
 file 'lambda'.

2. Locally revert the C part of r1841731 and make sure the modified test
 still (correctly) fails.  (That revision both added the test and
 fixed the bug the test checks for.)


So it looks sufficient to me.


Cool.  Will you perchance have time to do this?  No worries if not.


Yes, I'll do it on FreeBSD on tonight or tomorrow night (in JST :)).
However I think it is also need to test for each 1 and 2 on Windows,
because r1841736 and r1841743 also were attempt to fix this test
on Windows, with Python 2.


Also, what about the svnadmin_tests.py patch you posted upthread?  Is
there a reason not to go ahead and commit it to trunk?  (and even nominate
it for backport in 1.13.x/STATUS)


I think that is no probrem.

Cheers,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-14 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Sun, 13 Oct 2019 04:01 +00:00:
> On 2019/10/13 7:24, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Sat, 12 Oct 2019 03:01 +00:00:
> >> If textual comparison is sufficient here, it is right to open file
> >> text mode (with suitable, unified set of `encoding', `errors', and 
> >> `newline'
> >> parameter). Otherwise, if strict comparison is needed, we must avoid 
> >> unwanted,
> >> not one-on-one corresponding conversion from bytes to str applied by 
> >> Python.
> >> In the latter case, it may be rather incorrect to use
> >> compare_and_display_lines().
> > 
> > Good question.  I suspect textual comparison would suffice here, because
> > this is a tree conflicts test, not a keywords semantics test, and the
> > test case seems to revolve around the tree changes, not around the
> > newline characters.
> 
> I see. Now I agree it would suffice here.
>   
> > So, how about:
> > 
> > 1. Make the test use non-binary mode for changing and reading the
> > file 'lambda'.
> > 
> > 2. Locally revert the C part of r1841731 and make sure the modified test
> > still (correctly) fails.  (That revision both added the test and
> > fixed the bug the test checks for.)
> 
> So it looks sufficient to me.

Cool.  Will you perchance have time to do this?  No worries if not.

Also, what about the svnadmin_tests.py patch you posted upthread?  Is
there a reason not to go ahead and commit it to trunk?  (and even nominate
it for backport in 1.13.x/STATUS)

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-12 Thread Nathan Hartman
On Fri, Oct 11, 2019 at 6:52 PM Daniel Shahaf 
wrote:

> Nathan, I appreciate the intent, but Yasuhito and I are both familiar with
> the
> semantics of str and bytes objects in Python


I misread the earlier message. Apologies for the noise.


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-12 Thread Yasuhito FUTATSUKI

On 2019/10/13 7:24, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Sat, 12 Oct 2019 03:01 +00:00:

If textual comparison is sufficient here, it is right to open file
text mode (with suitable, unified set of `encoding', `errors', and `newline'
parameter). Otherwise, if strict comparison is needed, we must avoid unwanted,
not one-on-one corresponding conversion from bytes to str applied by Python.
In the latter case, it may be rather incorrect to use
compare_and_display_lines().


Good question.  I suspect textual comparison would suffice here, because
this is a tree conflicts test, not a keywords semantics test, and the
test case seems to revolve around the tree changes, not around the
newline characters.


I see. Now I agree it would suffice here.
 

So, how about:

1. Make the test use non-binary mode for changing and reading the
file 'lambda'.

2. Locally revert the C part of r1841731 and make sure the modified test
still (correctly) fails.  (That revision both added the test and
fixed the bug the test checks for.)


So it looks sufficient to me.

Thanks,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-12 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Sat, 12 Oct 2019 03:01 +00:00:
> If textual comparison is sufficient here, it is right to open file
> text mode (with suitable, unified set of `encoding', `errors', and `newline'
> parameter). Otherwise, if strict comparison is needed, we must avoid unwanted,
> not one-on-one corresponding conversion from bytes to str applied by Python.
> In the latter case, it may be rather incorrect to use
> compare_and_display_lines().

Good question.  I suspect textual comparison would suffice here, because
this is a tree conflicts test, not a keywords semantics test, and the
test case seems to revolve around the tree changes, not around the
newline characters.

So, how about:

1. Make the test use non-binary mode for changing and reading the
   file 'lambda'.

2. Locally revert the C part of r1841731 and make sure the modified test
   still (correctly) fails.  (That revision both added the test and
   fixed the bug the test checks for.)

Alternatively, we could make the test verify the contents of 'lambda' in binary
mode without using compare_and_display_lines.

Makes sense?

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Yasuhito FUTATSUKI

On 2019-10-12 07:47, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Sat, Oct 12, 2019 at 05:31:53 +0900:

Yes, it will fix local_missing_dir_endless_loop() itself correctly.
But the stack trace before fix indicate there is at least one problem
in svntest.verify.compare_and_display_lines().

Assume the file contents is broken here. This situation can be simulate
by patch like:

Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1544,7 +1544,7 @@
contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
svntest.verify.compare_and_display_lines(
  "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
-[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+[ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
contents)
  ###


then we will got fails.log, contains stack trace for unexpected exception
within the code to construct log message.

[[[
W: A1/B/lambda has unexpectected contents
W: EXPECTED svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda 
(match_all=True):
W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
Traceback (most recent call last):
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py",
 line 1931, in run
 rc = self.pred.run(sandbox)
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py",
 line 178, in run
 result = self.func(sandbox)
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/tree_conflict_tests.py",
 line 1547, in local_missing_dir_endless_loop
 [ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
contents)
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 503, in compare_and_display_lines
 expected.display_differences(message, label, actual)
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 154, in display_differences
 display_lines(message, self.expected, actual, e_label, label)
   File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 474, in display_lines
 logger.warn('| ' + x.rstrip())
TypeError: can only concatenate str (not "bytes") to str
FAIL:  tree_conflict_tests.py 26: endless loop when resolving local-missing dir
]]]

This is caused by mixing bytes object drived from file contents and str
object to construct log message.


I agree: this FAIL indicates str and bytes are mixed.  My question is:
Why do you think svntest.verify.compare_and_display_lines() needs to be
changed?  That function's names implies it deals with text files, so,
why should compare_and_display_lines() support the case that its third
and fourth parameters (EXPECTED and ACTUAL) are both lists of bytes objects?

In other words, why would changing «'rb'» to «'r'» on line 1544 —
without changing the str literals to bytes literals on line 1547 — not
be a correct solution?


Ah, I thought strict comparison is needed here because of raw mode file I/O.

Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1518,7 +1518,7 @@
   sbox.simple_move('A/B', 'A/B2')
   sbox.simple_commit()
   sbox.simple_update()
-  main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more 
content.\n")
+  main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more 
content.\r\n")
   sbox.simple_commit()
   sbox.simple_update()
 
@@ -1544,7 +1544,7 @@

   contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
   svntest.verify.compare_and_display_lines(
 "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
-[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+[ b"This is the file 'lambda'.\n", b"This is more content.\n"], contents)
 
 
 ###


Above patch will make local_missing_dir_endless_loop test fail because of
strictness of comparison. However,

Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1518,7 +1518,7 @@
   sbox.simple_move('A/B', 'A/B2')
   sbox.simple_commit()
   sbox.simple_update()
-  main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more 
content.\n")
+ 

Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Daniel Shahaf
Nathan Hartman wrote on Fri, Oct 11, 2019 at 16:50:39 -0400:
> On Fri, Oct 11, 2019 at 4:32 PM Yasuhito FUTATSUKI  
> wrote:
> > This is caused by mixing bytes object drived from file contents and str
> > object to construct log message.
> 
> Does something like this answer help:
> 
> https://stackoverflow.com/questions/31058055/how-do-i-convert-a-python-3-byte-string-variable-into-a-regular-string/31060836
> 
> Something like:
> str(bytes_string, 'utf-8')

Nathan, I appreciate the intent, but Yasuhito and I are both familiar with the
semantics of str and bytes objects in Python 3.  We're not asking what the
difference between bytes and str is, or how to work with them; we are simply
trying to resolve two particular test failures, in svnadmin_ and
tree_conflict_tests.py.  Specifically, we're trying to determine whether file
contents should be handled as str or as bytes, both in the test function and in
the test framework.

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Sat, Oct 12, 2019 at 05:31:53 +0900:
> Yes, it will fix local_missing_dir_endless_loop() itself correctly.
> But the stack trace before fix indicate there is at least one problem
> in svntest.verify.compare_and_display_lines().
> 
> Assume the file contents is broken here. This situation can be simulate
> by patch like:
> 
> Index: subversion/tests/cmdline/tree_conflict_tests.py
> ===
> --- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
> +++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
> @@ -1544,7 +1544,7 @@
>contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
>svntest.verify.compare_and_display_lines(
>  "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
> -[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
> +[ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
> contents)
>  ###
> 
> 
> then we will got fails.log, contains stack trace for unexpected exception
> within the code to construct log message.
> 
> [[[
> W: A1/B/lambda has unexpectected contents
> W: EXPECTED svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda 
> (match_all=True):
> W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
> Traceback (most recent call last):
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py",
>  line 1931, in run
> rc = self.pred.run(sandbox)
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py",
>  line 178, in run
> result = self.func(sandbox)
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/tree_conflict_tests.py",
>  line 1547, in local_missing_dir_endless_loop
> [ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
> contents)
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
>  line 503, in compare_and_display_lines
> expected.display_differences(message, label, actual)
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
>  line 154, in display_differences
> display_lines(message, self.expected, actual, e_label, label)
>   File 
> "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
>  line 474, in display_lines
> logger.warn('| ' + x.rstrip())
> TypeError: can only concatenate str (not "bytes") to str
> FAIL:  tree_conflict_tests.py 26: endless loop when resolving local-missing 
> dir
> ]]]
> 
> This is caused by mixing bytes object drived from file contents and str
> object to construct log message.

I agree: this FAIL indicates str and bytes are mixed.  My question is:
Why do you think svntest.verify.compare_and_display_lines() needs to be
changed?  That function's names implies it deals with text files, so,
why should compare_and_display_lines() support the case that its third
and fourth parameters (EXPECTED and ACTUAL) are both lists of bytes objects?

In other words, why would changing «'rb'» to «'r'» on line 1544 —
without changing the str literals to bytes literals on line 1547 — not
be a correct solution?  

Hope I'm clearer this time.  If not, I'd be happy to clarify.

Cheers,

Daniel


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Nathan Hartman
On Fri, Oct 11, 2019 at 4:32 PM Yasuhito FUTATSUKI  wrote:
> This is caused by mixing bytes object drived from file contents and str
> object to construct log message.

Does something like this answer help:

https://stackoverflow.com/questions/31058055/how-do-i-convert-a-python-3-byte-string-variable-into-a-regular-string/31060836

Something like:
str(bytes_string, 'utf-8')


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Yasuhito FUTATSUKI

On 2019-10-12 02:56, Daniel Shahaf wrote:

Yasuhito FUTATSUKI wrote on Fri, Oct 11, 2019 at 16:35:19 +0900:



The latter also can be fixed by fix_tree_conflict_tests_patch.txt
at least so that the test can be passed on Python 3. However, from above
stack trace, I think it is incomplete because there still exists some sort of
confusion between bytes and str in subversion/tests/cmdline/svntest/*.py

Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1544,7 +1544,7 @@
contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
svntest.verify.compare_and_display_lines(
  "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
-[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+[ b"This is the file 'lambda'.\n", b"This is more content.\n"], contents)


Why do you think this is incomplete?  The open() call uses mode='rb', so
«contents» will be set to an array of bytes objects, so it'll need to be
compared to an array of bytes objects.  Which is to say, this patch, too, looks
correct to me.


Yes, it will fix local_missing_dir_endless_loop() itself correctly.
But the stack trace before fix indicate there is at least one problem
in svntest.verify.compare_and_display_lines().

Assume the file contents is broken here. This situation can be simulate
by patch like:

Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1544,7 +1544,7 @@
   contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
   svntest.verify.compare_and_display_lines(
 "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
-[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+[ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
contents)
 
 
 ###



then we will got fails.log, contains stack trace for unexpected exception
within the code to construct log message.

[[[
W: A1/B/lambda has unexpectected contents
W: EXPECTED svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda 
(match_all=True):
W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
Traceback (most recent call last):
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py",
 line 1931, in run
rc = self.pred.run(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py",
 line 178, in run
result = self.func(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/tree_conflict_tests.py",
 line 1547, in local_missing_dir_endless_loop
[ b"This is the file 'lambda'.\n", b"This is not more content.\n"], 
contents)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 503, in compare_and_display_lines
expected.display_differences(message, label, actual)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 154, in display_differences
display_lines(message, self.expected, actual, e_label, label)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 474, in display_lines
logger.warn('| ' + x.rstrip())
TypeError: can only concatenate str (not "bytes") to str
FAIL:  tree_conflict_tests.py 26: endless loop when resolving local-missing dir
]]]

This is caused by mixing bytes object drived from file contents and str
object to construct log message.

Cheers,
--
Yasuhito FUTATSUKI 


Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Daniel Shahaf
Yasuhito FUTATSUKI wrote on Fri, Oct 11, 2019 at 16:35:19 +0900:
> On 2019-10-11 06:45, Daniel Shahaf wrote:
> ...
> >  From autogen.sh to 'make check', all steps pass with 
> > PYTHON=/usr/bin/python3.
> 
> There were two failures in 1.11 on tests.
> 
> https://lists.apache.org/thread.html/366e77c7443a0c575b47de150a2c394e1e685fde8887c805a656a33d@%3Cusers.subversion.apache.org%3E
> 
> and it seems they still exist in trunk.

Sorry, my bad.  I did see them in my test run too, but incorrectly determined
they were an artifact of my build environment.

> The former failure can be fixed by attached patch, 
> fix_svnadmin_tests_patch.txt

> Index: subversion/tests/cmdline/svnadmin_tests.py
> ===
> --- subversion/tests/cmdline/svnadmin_tests.py(revision 1868264)
> +++ subversion/tests/cmdline/svnadmin_tests.py(working copy)
> @@ -3859,7 +3859,7 @@
>   sbox.repo_url)
>  
>dump_lines = svntest.actions.run_and_verify_dump(sbox.repo_dir)
> -  assert propval + '\n' in dump_lines
> +  assert propval.encode() + b'\n' in dump_lines

+1 to commit.

> The latter also can be fixed by fix_tree_conflict_tests_patch.txt
> at least so that the test can be passed on Python 3. However, from above
> stack trace, I think it is incomplete because there still exists some sort of
> confusion between bytes and str in subversion/tests/cmdline/svntest/*.py
> 
> Index: subversion/tests/cmdline/tree_conflict_tests.py
> ===
> --- subversion/tests/cmdline/tree_conflict_tests.py   (revision 1868264)
> +++ subversion/tests/cmdline/tree_conflict_tests.py   (working copy)
> @@ -1544,7 +1544,7 @@
>contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
>svntest.verify.compare_and_display_lines(
>  "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
> -[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
> +[ b"This is the file 'lambda'.\n", b"This is more content.\n"], contents)

Why do you think this is incomplete?  The open() call uses mode='rb', so
«contents» will be set to an array of bytes objects, so it'll need to be
compared to an array of bytes objects.  Which is to say, this patch, too, looks
correct to me.

Cheers,

Daniel


Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October))

2019-10-11 Thread Yasuhito FUTATSUKI

On 2019-10-11 06:45, Daniel Shahaf wrote:
...

 From autogen.sh to 'make check', all steps pass with PYTHON=/usr/bin/python3.


There were two failures in 1.11 on tests.
 
https://lists.apache.org/thread.html/366e77c7443a0c575b47de150a2c394e1e685fde8887c805a656a33d@%3Cusers.subversion.apache.org%3E


and it seems they still exist in trunk.

on FreeBSD 11.1, Python 3.7.0, Subversion trunk@1868235:
[[[
W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
Traceback (most recent call last):
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py",
 line 1931, in run
rc = self.pred.run(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py",
 line 178, in run
result = self.func(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svnadmin_tests.py",
 line 3862, in dump_no_canonicalize_svndate
assert propval + '\n' in dump_lines
AssertionError
FAIL:  svnadmin_tests.py 69: svnadmin dump shouldn't canonicalize svn:date
]]]

[[[
W: A1/B/lambda has unexpectected contents
W: EXPECTED svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda 
(match_all=True):
W: | This is the file 'lambda'.
W: | This is more content.
W: ACTUAL svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda:
W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
Traceback (most recent call last):
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py",
 line 1931, in run
rc = self.pred.run(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py",
 line 178, in run
result = self.func(sandbox)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/tree_conflict_tests.py",
 line 1547, in local_missing_dir_endless_loop
[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 503, in compare_and_display_lines
expected.display_differences(message, label, actual)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 154, in display_differences
display_lines(message, self.expected, actual, e_label, label)
  File 
"/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py",
 line 478, in display_lines
logger.warn('| ' + x.rstrip())
TypeError: can only concatenate str (not "bytes") to str
FAIL:  tree_conflict_tests.py 26: endless loop when resolving local-missing dir
]]]

The former failure can be fixed by attached patch, fix_svnadmin_tests_patch.txt

The latter also can be fixed by fix_tree_conflict_tests_patch.txt
at least so that the test can be passed on Python 3. However, from above
stack trace, I think it is incomplete because there still exists some sort of
confusion between bytes and str in subversion/tests/cmdline/svntest/*.py

Cheers,
--
Yasuhito FUTATSUKI 
Index: subversion/tests/cmdline/svnadmin_tests.py
===
--- subversion/tests/cmdline/svnadmin_tests.py  (revision 1868264)
+++ subversion/tests/cmdline/svnadmin_tests.py  (working copy)
@@ -3859,7 +3859,7 @@
  sbox.repo_url)
 
   dump_lines = svntest.actions.run_and_verify_dump(sbox.repo_dir)
-  assert propval + '\n' in dump_lines
+  assert propval.encode() + b'\n' in dump_lines
 
 def check_recover_prunes_rep_cache(sbox, enable_rep_sharing):
   """Check 'recover' prunes the rep-cache while enable-rep-sharing is
Index: subversion/tests/cmdline/tree_conflict_tests.py
===
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1544,7 +1544,7 @@
   contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
   svntest.verify.compare_and_display_lines(
 "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
-[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+[ b"This is the file 'lambda'.\n", b"This is more content.\n"], contents)
 
 
 ###