Supported SWIG version on swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))
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))
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)))
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)))
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)))
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)))
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)))
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)))
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)))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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))
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) ###