You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jun Omae <ju...@gmail.com> on 2022/10/02 14:17:07 UTC

Re: svn commit: r1904198 - in /subversion/trunk/build: ac-macros/swig.m4 generator/gen_win_dependencies.py

Hi Yasuhito,

On 2022/09/22 2:45, futatuki@apache.org wrote:
> Author: futatuki
> Date: Wed Sep 21 17:45:48 2022
> New Revision: 1904198
> 
> URL: http://svn.apache.org/viewvc?rev=1904198&view=rev
> Log:
> swig-py: build: Don't pass deperecated options to SWIG >= 4.1.0 release
> ...> 
> Modified: subversion/trunk/build/ac-macros/swig.m4
> URL: http://svn.apache.org/viewvc/subversion/trunk/build/ac-macros/swig.m4?rev=1904198&r1=1904197&r2=1904198&view=diff
> ==============================================================================
> --- subversion/trunk/build/ac-macros/swig.m4 (original)
> +++ subversion/trunk/build/ac-macros/swig.m4 Wed Sep 21 17:45:48 2022
> @@ -182,22 +182,29 @@ suitable Python interpreter is not found
>                  if test "$SWIG_VERSION" -ge "300010"; 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 -py3 -nofastunpack -modern"
> +                  elif test "$SWIG_VERSION" -lt "401000"; then 
> +                    SWIG_PY_OPTS="-python -py3 -nofastunpack"
> +                  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-a-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
> -                if test "$SWIG_VERSION" -lt "400000"; then
> -                  SWIG_PY_OPTS="-python -py3 -nofastunpack -modern"
> -                else
> -                  SWIG_PY_OPTS="-python -py3 -nofastunpack"
> -                fi
>                else
>                  if test "$SWIG_VERSION" -lt "400000"; then
>                    SWIG_PY_OPTS="-python -classic"
>                    dnl SWIG Python bindings successfully configured, clear the error message dnl
>                    SWIG_PY_ERRMSG=""
>                  else
> -                  SWIG_PY_OPTS="-python -nofastunpack"
> +                  SWIG_PY_OPTS="-no-such-a-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])
>                  fi
> 
> Modified: subversion/trunk/build/generator/gen_win_dependencies.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/build/generator/gen_win_dependencies.py?rev=1904198&r1=1904197&r2=1904198&view=diff
> ==============================================================================
> --- subversion/trunk/build/generator/gen_win_dependencies.py (original)
> +++ subversion/trunk/build/generator/gen_win_dependencies.py Wed Sep 21 17:45:48 2022
> @@ -1059,8 +1059,13 @@ class GenDependenciesBase(gen_base.Gener
>          return
>        if self.swig_version < (4, 0, 0):
>          opts = "-python -py3 -nofastunpack -modern"
> -      else:
> +      elif self.swig_version < (4, 1, 0):
>          opts = "-python -py3 -nofastunpack"
> +      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 show_warnings:
> 

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. 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
and re-generate *.py and *.c files).

[[[
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:
]]]

-- 
Jun Omae <ju...@gmail.com> (大前 潤)


Re: svn commit: r1904198 - in /subversion/trunk/build: ac-macros/swig.m4 generator/gen_win_dependencies.py

Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
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 <fu...@yf.bsclub.org>