Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>]

On 1/20/19 5:50 PM, Yasuhito FUTATSUKI wrote:
> Patch updated.

I've commited it with modifications below, as r1853592.

* Add support to convert py_file from unicode file name in
  svn_swig_py_make_file()
* Revise test case for svn_stream_write() in python/tests/core.py

Thanks,
-- 
Yasuhito FUTATSUKI

Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>]

On 2/20/19 11:05 PM, Troy Curtis Jr wrote:
> On Sun, Feb 17, 2019, 10:32 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> 
>> Yasuhito FUTATSUKI wrote on Sun, 17 Feb 2019 06:13 +00:00:
>>> On 2/17/19 9:04 AM, Daniel Shahaf wrote:

>>> * If we want to install py2 bindings and py3 bindings at the same time,
>>>     we must rename libsvn_swig_py library for py3.
>>
>> Good question, but I don't know the answer.
>>
> 
> Do we think we want to address this directly, or rely on packagers to do
> whatever makes since for their environment? Perhaps we should just include
> an easy, optional, library name override option?

I think the former is better to save users who build from source from
unintentionally overwrite. It is also convenient to distinguish what
version of Python has been used if target Python major/minor version
number is added to libsvn_swig_py for py3 just like libpythonX.Ym is.

However, it doesn't intend to separate build tree immediately. To build
py3 bindings in addition to py2 bindings (or contrary), as we just do
currently, (1) clean swig-py stuff, (2) configure or gen-make.py again,
(3) and then rebuild swig-py.

Isn't it sufficient to include an easy, optional, library name override
option with different default value between py2 and py3?

Cheers,
-- 
Yasuhito FUTATSUKI

Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Troy Curtis Jr <tr...@gmail.com>]

On Sun, Feb 17, 2019, 10:32 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:

> Yasuhito FUTATSUKI wrote on Sun, 17 Feb 2019 06:13 +00:00:
> > On 2/17/19 9:04 AM, Daniel Shahaf wrote:
> >
> > > I was wondering what are the blockers for merging swig-py3 into trunk?
> > > The only item in BRANCH-README is "Fix a compile error on windows with
> > > py3", but I don't even know if that's a current issue or an old one
> that has
> > > since been fixed.
> >
> > I'm sorry I'm not sure the compile error still exists or not. (I don't
> > have Windows build environment yet)
> >
>
> There's no need to apologize; we don't expect every developer to be able
> to build on every platform.  If you are able to set up a Windows build
> environment that would be very helpful, of course, but if not, a Windows
> developer can do that.
>
> If you just need a quick tests run, you can also trigger buildbot builds
> of the swig-py3 branch on Windows by joining #svn-dev on IRC and
> issueing an appropriate command to buildbot's IRC bot.
>

The Py3 windows build issue still exists. I have started rebuilding my
Windows Dev environment to see about addressing it, but I do not yet have a
functioning build system. :P

It does sound like there may be a Py2 issue introduced now as well. I could
see the Py2 issue being a blocker for a trunk merge, but presumably the Py3
wouldn't necessarily be one.


> > The only thing I know about it, my patch committed on r1849784 might
> break
> > Windows build because of "PY3" macro for swig which added only Unix/Linux
> > build. However, it might not be a compile error, but might produce
> improper
> > codes under py3 environment, and this might be dissolved on r1853592,
> > for "PY3" macro in swig typemaps no longer used.
> >
>
> OK.
>
> >
> > I think there are still some other points need to look over.
> >
> > * Do scrpts under tools/ work on py3?
>
> Good call: the branch should not cause regressions in tools/ scripts
> that use the SWIG bindings.  That said, I would assume that Python
> scripts that don't use the swig bindings are orthogonal to the branch;
> they _should_ be ported to Python 3, but that work can happen on trunk,
> before or after the merge.
>
> > * If we want to install py2 bindings and py3 bindings at the same time,
> >    we must rename libsvn_swig_py library for py3.
>
> Good question, but I don't know the answer.
>

Do we think we want to address this directly, or rely on packagers to do
whatever makes since for their environment? Perhaps we should just include
an easy, optional, library name override option?


> Thanks,
>
> Daniel
>

Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Daniel Shahaf <d....@daniel.shahaf.name>]

Yasuhito FUTATSUKI wrote on Sun, 17 Feb 2019 06:13 +00:00:
> On 2/17/19 9:04 AM, Daniel Shahaf wrote:
> 
> > I was wondering what are the blockers for merging swig-py3 into trunk?
> > The only item in BRANCH-README is "Fix a compile error on windows with
> > py3", but I don't even know if that's a current issue or an old one that has
> > since been fixed.
> 
> I'm sorry I'm not sure the compile error still exists or not. (I don't
> have Windows build environment yet)
> 

There's no need to apologize; we don't expect every developer to be able
to build on every platform.  If you are able to set up a Windows build
environment that would be very helpful, of course, but if not, a Windows
developer can do that.

If you just need a quick tests run, you can also trigger buildbot builds
of the swig-py3 branch on Windows by joining #svn-dev on IRC and
issueing an appropriate command to buildbot's IRC bot.

> The only thing I know about it, my patch committed on r1849784 might break
> Windows build because of "PY3" macro for swig which added only Unix/Linux
> build. However, it might not be a compile error, but might produce improper
> codes under py3 environment, and this might be dissolved on r1853592,
> for "PY3" macro in swig typemaps no longer used.
> 

OK.

> 
> I think there are still some other points need to look over.
> 
> * Do scrpts under tools/ work on py3?

Good call: the branch should not cause regressions in tools/ scripts
that use the SWIG bindings.  That said, I would assume that Python
scripts that don't use the swig bindings are orthogonal to the branch;
they _should_ be ported to Python 3, but that work can happen on trunk,
before or after the merge.

> * If we want to install py2 bindings and py3 bindings at the same time,
>    we must rename libsvn_swig_py library for py3.

Good question, but I don't know the answer.

Thanks,

Daniel

Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>]

On 2/17/19 9:04 AM, Daniel Shahaf wrote:

> I was wondering what are the blockers for merging swig-py3 into trunk?
> The only item in BRANCH-README is "Fix a compile error on windows with
> py3", but I don't even know if that's a current issue or an old one that has
> since been fixed.

I'm sorry I'm not sure the compile error still exists or not. (I don't
have Windows build environment yet)

The only thing I know about it, my patch committed on r1849784 might break
Windows build because of "PY3" macro for swig which added only Unix/Linux
build. However, it might not be a compile error, but might produce improper
codes under py3 environment, and this might be dissolved on r1853592,
for "PY3" macro in swig typemaps no longer used.


I think there are still some other points need to look over.

* Do scrpts under tools/ work on py3?
* If we want to install py2 bindings and py3 bindings at the same time,
   we must rename libsvn_swig_py library for py3.

Cheers,
-- 
Yasuhito FUTATSUKI

Re: [PATCH]On branch swig-py3: accept both of bytes/str for inputchar * args

[Daniel Shahaf <d....@daniel.shahaf.name>]

Yasuhito FUTATSUKI wrote on Sat, 16 Feb 2019 07:37 +00:00:
> On 1/20/19 5:50 PM, Yasuhito FUTATSUKI wrote:
> > Patch updated.
> 
> I've commited it with modifications below, as r1853592.

Thanks!

I was wondering what are the blockers for merging swig-py3 into trunk?
The only item in BRANCH-README is "Fix a compile error on windows with
py3", but I don't even know if that's a current issue or an old one that has
since been fixed.

Cheers,

Daniel