On 2020/12/02 4:24, Branko Čibej wrote:
> On 01.12.2020 17:34, Yasuhito FUTATSUKI wrote:
>> On 2020/11/17 11:01, Yasuhito FUTATSUKI wrote:
>>> On 2020/11/17 2:16, Daniel Shahaf wrote:
>>>> Yasuhito FUTATSUKI wrote on Sat, 14 Nov 2020 14:52 +0900:
>>>>> +++ subversion/bindings/swig/INSTALL (working copy)
>>>>> @@ -141,7 +141,15 @@
>>>>> - Make sure that Subversion's ./configure script sees your installed SWIG!
>>>>> + If you are using the distribution tarball and you want to use the language
>>>>> + bindings C source files shipped with it, you might need to pass the
>>>>> + --without-swig option to configure script to avoid detecting and checking
>>>>> + SWIG on your system. A Makefile generated by configure will prevent
>>>>> + building the language bindings if the configure script detect unsuitable
>>>>> + version of SWIG.
>>>> I don't dispute the accuracy of this paragraph, but I think this API
>>>> isn't autotools-idiomatic.
>>>>
>>>> Generally, I'd expect --without-foo to short-circuit the probe for foo
>>>> and assume foo isn't found; i.e., if my system has an unsuitable
>>>> version of foo, I'd the default behaviour (given neither --with-foo nor
>>>> --without-foo) and the behaviour given --without-foo to be identical:
>>>> namely, behave as though foo isn't available (even if /usr/bin/foo
>>>> exists and is perfectly suitable).
>>>>
>>>> In particular, if my system has an unsuitable version of swig,
>>>> I wouldn't expect passing --without-swig to change configure's behaviour.
>>> Probably what is wrong here is that the configure script accepts
>>> --with-swig | --without-swig options and checks it in release mode.
>>>
>>> We never clean SWIG generated language bindings C source files on
>>> clean-foo targets in release mode Makefile. extraclean-foo targets do it,
>>> but they are only parts of the extraclean target which also removes all
>>> release mode stuff. So users never use SWIG in release mode actually.
>>>
>>> That is, r1876662 is not correct.
>> To fix it, I tweaked the configure script again.
>>
>> The patch attached introduce "release mode" for configure generation.
>> With this patch, configure script generated by "autogen.sh --release"
>> does not have --with-swig|--without-swig option and never check SWIG
>> executable.
>>
>> Could anyone please review this?
>>
>> For backward compatibility to back port to 1.14.x, default for
>> --with-swig-perl, --with-swig-python, --with-swig-ruby are "auto",
>> i.e. search the target language, but I'd like to change them to "no" in
>> trunk, because I think those are optional feature.
>
>
> This makes sense, yes. More comments below.
Many thanks for the reviw. Then I've amended the patch.
(attached file, introduce-release-mode-configure-patch-v2.txt)
In addition to the items pointed out, I also added two modification:
* Clean up aclocal.m4 on local-extraclean target in Makefile.in.
* Move the task to generate aclocal.m4 earlier in autogen.sh, because
aclocal.m4 file is also (at least) used by autoheader.
Also, I make diffs for release mode block and non-release mode block
of SVN_DETERMINE_SWIG_OPTS from corresponding block before this patch
in build/ac-macros/swig.m4, to show how they are changed.
Non release mode:
[[[
diff -ubB ./SVN_DETERMINE_SWIG_OPTS_original_block.txt ./SVN_DETERMINE_SWIG_OPTS_non_release_block.txt
--- ./SVN_DETERMINE_SWIG_OPTS_original_block.txt 2020-12-04 01:55:39.289774000 +0900
+++ ./SVN_DETERMINE_SWIG_OPTS_non_release_block.txt 2020-12-04 06:21:30.416610000 +0900
@@ -1,8 +1,16 @@
+ # not in release mode
SWIG_PY_COMPILE="none"
SWIG_PY_LINK="none"
SWIG_PY_OPTS="none"
SWIG_PY_ERRMSG="check config.log for details"
- if test "$SWIG_PY_PYTHON" != "none"; then
+ if test "$SWIG_PY_PYTHON" = "none"; then
+ SWIG_PY_ERRMSG="You specfied not to build Python bindings or \
+suitable Python interpreter is not found."
+ else
+ if test "$SWIG" = "none"; then
+ AC_MSG_WARN([You specified to build SWIG Python bindings, but SWIG is not found.])
+ SWIG_PY_ERRMSG="SWIG is need to build SWIG Python bindings, but it is not found."
+ else
AC_MSG_NOTICE([Configuring python swig binding])
AC_CACHE_CHECK([for Python includes], [ac_cv_python_includes],[
@@ -49,9 +57,6 @@
])
SWIG_PY_LIBS="`SVN_REMOVE_STANDARD_LIB_DIRS($ac_cv_python_libs)`"
- if test "$SWIG" = "none"; then
- SWIG_PY_ERRMSG=""
- else
# Look more closely at the SWIG and Python versions to
# determine SWIG_PY_OPTS. We can skip this if we already
# have the SWIG-generated files.
@@ -93,7 +97,14 @@
fi
SWIG_PL_ERRMSG="check config.log for details"
- if test "$SWIG_PL_PERL" != "none"; then
+ if test "$SWIG_PL_PERL" = "none"; then
+ SWIG_PL_ERRMSG="You specfied not to build Perl bindings or \
+suitable Perl interpreter is not found."
+ else
+ if test "$SWIG" = "none"; then
+ AC_MSG_WARN([You specified to build SWIG Perl bindings, but SWIG is not found.])
+ SWIG_PL_ERRMSG="SWIG is need to build SWIG Perl bindings, but it is not found."
+ else
AC_MSG_CHECKING([perl version])
dnl Note that the q() bit is there to avoid unbalanced brackets
dnl which m4 really doesn't like.
@@ -110,11 +121,19 @@
AC_MSG_WARN([perl bindings require perl 5.8.0 or newer.])
fi
fi
+ fi
SWIG_RB_COMPILE="none"
SWIG_RB_LINK="none"
SWIG_RB_ERRMSG="check config.log for details"
- if test "$SWIG_RB_RUBY" != "none"; then
+ if test "$SWIG_RB_RUBY" = "none"; then
+ SWIG_RB_ERRMSG="You specfied not to build Ruby bindings or \
+suitable Ruby interpreter is not found."
+ else
+ if test "$SWIG" = "none"; then
+ AC_MSG_WARN([You specified to build SWIG Ruby bindings, but SWIG is not found.])
+ SWIG_RB_ERRMSG="SWIG is need to build SWIG Ruby bindings, but it is not found."
+ else
if test x"$SWIG_VERSION" = x"3""00""008"; then
# Use a local variable to escape the '#' sign.
ruby_swig_issue_602='https://subversion.apache.org/docs/release-notes/1.11#ruby-swig-issue-602'
@@ -234,4 +253,5 @@
dnl SWIG Ruby bindings successfully configured, clear the error message
SWIG_RB_ERRMSG=""
+ fi
fi
]]]
Release mode:
[[[
diff -ubB ./SVN_DETERMINE_SWIG_OPTS_original_block.txt ./SVN_DETERMINE_SWIG_OPTS_release_block.txt
--- ./SVN_DETERMINE_SWIG_OPTS_original_block.txt 2020-12-04 01:55:39.289774000 +0900
+++ ./SVN_DETERMINE_SWIG_OPTS_release_block.txt 2020-12-04 06:21:56.977529000 +0900
@@ -1,8 +1,12 @@
+ # in release mode
SWIG_PY_COMPILE="none"
SWIG_PY_LINK="none"
SWIG_PY_OPTS="none"
SWIG_PY_ERRMSG="check config.log for details"
- if test "$SWIG_PY_PYTHON" != "none"; then
+ if test "$SWIG_PY_PYTHON" = "none"; then
+ SWIG_PY_ERRMSG="You specfied not to build Python bindings or \
+suitable Python interpreter is not found."
+ else
AC_MSG_NOTICE([Configuring python swig binding])
AC_CACHE_CHECK([for Python includes], [ac_cv_python_includes],[
@@ -49,51 +53,17 @@
])
SWIG_PY_LIBS="`SVN_REMOVE_STANDARD_LIB_DIRS($ac_cv_python_libs)`"
- if test "$SWIG" = "none"; then
SWIG_PY_ERRMSG=""
- else
- # Look more closely at the SWIG and Python versions to
- # determine SWIG_PY_OPTS. We can skip this if we already
- # have the SWIG-generated files.
- AC_CACHE_CHECK([for Python >= 3], [ac_cv_python_is_py3],[
- ac_cv_python_is_py3="no"
- $SWIG_PY_PYTHON -c 'import sys; sys.exit(0x3000000 > sys.hexversion)' && \
- ac_cv_python_is_py3="yes"
- ])
-
- if test "$ac_cv_python_is_py3" = "yes"; then
- if test "$SWIG_VERSION" -ge "300010"; then
- dnl SWIG Python bindings successfully configured, clear the error message dnl
- SWIG_PY_ERRMSG=""
- else
- 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_ERRMSG="SWIG version is not suitable"
- AC_MSG_WARN([Subversion Python bindings for Python 2 require 1.3.24 <= SWIG < 4.0.0])
fi
fi
- fi
- fi
- fi
- fi
- fi
-
SWIG_PL_ERRMSG="check config.log for details"
- if test "$SWIG_PL_PERL" != "none"; then
+ if test "$SWIG_PL_PERL" = "none"; then
+ SWIG_PL_ERRMSG="You specfied not to build Perl bindings or \
+suitable Perl interpreter is not found."
+ else
AC_MSG_CHECKING([perl version])
dnl Note that the q() bit is there to avoid unbalanced brackets
dnl which m4 really doesn't like.
@@ -114,12 +84,10 @@
SWIG_RB_COMPILE="none"
SWIG_RB_LINK="none"
SWIG_RB_ERRMSG="check config.log for details"
- if test "$SWIG_RB_RUBY" != "none"; then
- if test x"$SWIG_VERSION" = x"3""00""008"; then
- # Use a local variable to escape the '#' sign.
- ruby_swig_issue_602='https://subversion.apache.org/docs/release-notes/1.11#ruby-swig-issue-602'
- AC_MSG_WARN([Ruby bindings are known not to support swig 3.0.8; see $ruby_swig_issue_602])
- fi
+ if test "$SWIG_RB_RUBY" = "none"; then
+ SWIG_RB_ERRMSG="You specfied not to build Ruby bindings or \
+suitable Ruby interpreter is not found."
+ else
rbconfig="$SWIG_RB_RUBY -rrbconfig -e "
for var_name in arch archdir CC LDSHARED DLEXT LIBS LIBRUBYARG \
]]]
Thanks,
--
Yasuhito FUTATSUKI <futatuki_at_yf.bsclub.org>
Received on 2020-12-03 22:41:49 CET