Hi, thank you for the follow up.

On 2022/10/02 23:17, Jun Omae wrote:

> SWIG 3.0.x/4.0.x with -py3 option generates function annotations in
> libsvn/*.py. E.g.:
> 
> [[[
>  144 def apr_initialize() -> "apr_status_t":
>  145     """apr_initialize() -> apr_status_t"""
>  146     return _core.apr_initialize()
> ]]]
> 
> However, SWIG 4.1.0dev doesn't generate the function annotations by default.
> Passing python:annotations using -features option to SWIG 4.1.0dev generates
> the function annotations.
> 
> [[[
> --- a/build/ac-macros/swig.m4
> +++ b/build/ac-macros/swig.m4
> @@ -187,7 +187,7 @@ suitable Python interpreter is not found."
>                    elif test "$SWIG_VERSION" -lt "401000"; then
>                      SWIG_PY_OPTS="-python -py3 -nofastunpack"
>                    else
> -                    SWIG_PY_OPTS="-python -nofastunpack"
> +                    SWIG_PY_OPTS="-python -nofastunpack -features 
> python:annotations=c,python:annotations:novar"
>                    fi
>                    if test "$SWIG_VERSION" -gt "400002"; then
>                      AC_MSG_WARN([Subversion Python bindings may work,])
> 
> --- a/build/generator/gen_win_dependencies.py
> +++ b/build/generator/gen_win_dependencies.py
> @@ -1062,7 +1062,7 @@ class GenDependenciesBase(gen_base.GeneratorBase):
>        elif self.swig_version < (4, 1, 0):
>          opts = "-python -py3 -nofastunpack"
>        else:
> -        opts = "-python -nofastunpack"
> +        opts = "-python -nofastunpack -features 
> python:annotations=c,python:annotations:novar"
>        if show_warnings and self.swig_version > (4, 0, 2):
>          print("WARNING: Subversion Python bindings may work,\n"
>                "but we didn't check with this SWIG version.")
> ]]]
> 
> But I don't think the function annotations are useful.

Sure. Any tool can't check types of argments with them. And we can see
the types of arguments on C API in docstring.

>                                                        We could remove -py3
> from options for SWIG 3.0.x/4.0.x with Python 3. Removing -py3 option makes
> generated *.py and *.c files available for both Python 2 and 3 without
> re-generating (currently, Python 3 users remove bundled generated *.py files
                            ^^^^^^^^ Python 2?
> and re-generate *.py and *.c files).

Sounds good. Perhaps we'll use SWIG >= 4.0 when we'll make new release
tarballs, so only Python 2 bindings users which use the code relied on
classic style class need re-generate *.py and *.c intermediate source
files with SWIG < 4.0 (although I doubt that there really exist such
users).

We should update subversion/bindings/swig/INSTALL about this.

> [[[
> diff --git a/build/ac-macros/swig.m4 b/build/ac-macros/swig.m4
> index 167007d89..a7d4a266b 100644
> --- a/build/ac-macros/swig.m4
> +++ b/build/ac-macros/swig.m4
> @@ -183,32 +183,34 @@ suitable Python interpreter is not found."
>                    dnl SWIG Python bindings successfully configured, clear 
> the error message dnl
>                    SWIG_PY_ERRMSG=""
>                    if test "$SWIG_VERSION" -lt "400000"; then
> -                    SWIG_PY_OPTS="-python -py3 -nofastunpack -modern"
> -                  elif test "$SWIG_VERSION" -lt "401000"; then 
> -                    SWIG_PY_OPTS="-python -py3 -nofastunpack"
> +                    SWIG_PY_OPTS="-python -nofastunpack -modern"
>                    else
>                      SWIG_PY_OPTS="-python -nofastunpack"
>                    fi
> -                  if test "$SWIG_VERSION" -gt "400002"; then 
> -                    AC_MSG_WARN([Subversion Python bindings may work,])
> -                    AC_MSG_WARN([but we didn't check with this SWIG 
> version.])
> -                  fi
>                  else
>                    SWIG_PY_OPTS="-no-such-option" # fool proof
>                    SWIG_PY_ERRMSG="SWIG version is not suitable"
>                    AC_MSG_WARN([Subversion Python bindings for Python 3 
> require SWIG 3.0.10 or newer])
>                  fi
>                else
> -                if test "$SWIG_VERSION" -lt "400000"; then
> -                  SWIG_PY_OPTS="-python -classic"
> +                if test "$SWIG_VERSION" -ge "103024"; then
>                    dnl SWIG Python bindings successfully configured, clear 
> the error message dnl
>                    SWIG_PY_ERRMSG=""
> +                  if test "$SWIG_VERSION" -lt "400000"; then
> +                    SWIG_PY_OPTS="-python -classic"
> +                  else
> +                    SWIG_PY_OPTS="-python -nofastunpack"
> +                  fi
>                  else
>                    SWIG_PY_OPTS="-no-such-option" # fool proof
>                    SWIG_PY_ERRMSG="SWIG version is not suitable"
> -                  AC_MSG_WARN([Subversion Python bindings for Python 2 
> require 1.3.24 <= SWIG < 4.0.0])
> +                  AC_MSG_WARN([Subversion Python bindings for Python 2 
> require SWIG 1.3.24 or newer])
>                  fi
>                fi
> +              if test "$SWIG_VERSION" -gt "400002"; then
> +                AC_MSG_WARN([Subversion Python bindings may work,])
> +                AC_MSG_WARN([but we didn't check with this SWIG version.])
> +              fi
>              fi
>            fi
>          fi
> diff --git a/build/generator/gen_win_dependencies.py 
> b/build/generator/gen_win_dependencies.py
> index b76255444..1a9384a0b 100644
> --- a/build/generator/gen_win_dependencies.py
> +++ b/build/generator/gen_win_dependencies.py
> @@ -1058,20 +1058,21 @@ class GenDependenciesBase(gen_base.GeneratorBase):
>            print("WARNING: Subversion Python bindings for Python 3 require 
> SWIG 3.0.10 or newer")
>          return
>        if self.swig_version < (4, 0, 0):
> -        opts = "-python -py3 -nofastunpack -modern"
> -      elif self.swig_version < (4, 1, 0):
> -        opts = "-python -py3 -nofastunpack"
> +        opts = "-python -nofastunpack -modern"
>        else:
>          opts = "-python -nofastunpack"
> -      if show_warnings and self.swig_version > (4, 0, 2):
> -        print("WARNING: Subversion Python bindings may work,\n"
> -              "but we didn't check with this SWIG version.")
>      else:
> -      if not ((1, 3, 24) <= self.swig_version < (4, 0, 0)):
> +      if self.swig_version < (1, 3, 24):
>          if show_warnings:
> -          print("WARNING: Subversion Python bindings for Python 2 require 
> 1.3.24 <= SWIG < 4.0.0")
> +          print("WARNING: Subversion Python bindings for Python 2 require 
> SWIG 1.3.24 or newer")
>          return
> -      opts = "-python -classic"
> +      if self.swig_version < (4, 0, 0):
> +        opts = "-python -nofastunpack -classic"
> +      else:
> +        opts = "-python -nofastunpack"
> +    if show_warnings and self.swig_version > (4, 0, 2):
> +      print("WARNING: Subversion Python bindings may work,\n"
> +            "but we didn't check with this SWIG version.")
>  
>      self.user_macros.append(UserMacro("SWIG_PY_OPTS", opts))
>      self._libraries['python'] = SVNCommonLibrary('python', inc_dir, lib_dir, 
> None,
> diff --git a/subversion/bindings/swig/include/proxy.swg 
> b/subversion/bindings/swig/include/proxy.swg
> index 7d2d0dd03..5949c52e0 100644
> --- a/subversion/bindings/swig/include/proxy.swg
> +++ b/subversion/bindings/swig/include/proxy.swg
> @@ -75,7 +75,7 @@
>    _set_instance_attr = 
> _swig_setattr_nondynamic_instance_variable(object.__setattr__)
>  
>  %}
> -#elif defined(SWIGPYTHON_PY3)
> +#elif !defined(SWIG_PYTHON_CLASSIC)
>  %pythoncode %{
>    # SWIG classes generated with -modern do not define this variable
>    try:
> ]]]

It looks good to me.

... and for "./autogen.sh --release",

[[[
Index: build/generator/gen_make.py
===================================================================
--- build/generator/gen_make.py (revision 1904286)
+++ build/generator/gen_make.py (working copy)
@@ -511,8 +511,9 @@
     standalone.write('top_srcdir = .\n')
     standalone.write('top_builddir = .\n')
     standalone.write('SWIG = swig\n')
+    # Assume we are using SWIG >= 4.0 in release mode
     swig_py_opts = os.environ.get('SWIG_PY_OPTS',
-                                  '-python -py3 -nofastunpack -modern')
+                                  '-python -nofastunpack')
     standalone.write('SWIG_PY_OPTS = %s\n' % (swig_py_opts))
     standalone.write('PYTHON = ' + sys.executable + '\n')
     standalone.write('\n')
]]]


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

Reply via email to