You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Yasuhito FUTATSUKI <fu...@poem.co.jp> on 2020/05/14 10:51:49 UTC
[PATCH] Update swig INSTALL document for Python 2 bindings
Hi,
I overlooked that 'make clean-swig-py' doesn't remove SWIG generated
source files if build-output.mk is generated for release mode. It is
need to clean them to rebuild source files for Python 2 bindings.
So I want to update subversion/bindings/swig/INSTALL so that users don't
fall into this pitfall. Could anyone please make this better?
[[[
* subversion/bindings/swig/INSTALL
(Step 2: Build and Install Subversion.): Add description for optional
process needed to build Python 2 bindings.
Index: subversion/bindings/swig/INSTALL
===================================================================
--- subversion/bindings/swig/INSTALL (revision 1877407)
+++ subversion/bindings/swig/INSTALL (working copy)
@@ -141,6 +141,10 @@
See Subversion's own INSTALL file for details.
+ If you are using Subversion distribution tarball and want to build
+ Python bindings for Python 2, you should run 'sh autogen.sh' before run
+ ./configure script to rebuild build environment as non-release mode.
+
Make sure that Subversion's ./configure script sees your installed SWIG!
It tries to detect SWIG near the very end of its output.
]]]
Thanks,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] Update swig INSTALL document for Python 2 bindings
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yasuhito FUTATSUKI wrote on Wed, 20 May 2020 10:18 +0900:
> [[[
> * INSTALL (I.C.13): Add Note that non-release mode is requires to build
> SWIG Python 2 binding.
>
s/13/12/
s/^ SWIG/ SWIG/. Continuation line should be indented at least two
spaces. Most of us indent continuation lines three or four spaces, so
they are indented more deeply than the first letter of the filename.
s/is requires/is required/
s/binding/bindings/
s/to build/for building/. (This is subtler than the others. With "for
building", the sentence would mean what you intend it to mean. With "to
build", the sentence would be grammatical but would have a different
meaning: "is required to" would, in that case, be synonymous with "is
compelled to".)
> +++ INSTALL (working copy)
> @@ -506,7 +506,13 @@
> +++ subversion/bindings/swig/INSTALL (working copy)
> @@ -143,6 +143,7 @@
LGTM. +1 to commit.
Don't forget to nominate this for backport in STATUS if you'd like this
to be backported to 1.14.1. If you have questions, just ask.
Cheers,
Daniel
Re: [PATCH] Update swig INSTALL document for Python 2 bindings
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
Thank you for the review.
On 2020/05/14 22:27, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Thu, 14 May 2020 19:51 +0900:
>> Hi,
>>
>> I overlooked that 'make clean-swig-py' doesn't remove SWIG generated
>> source files if build-output.mk is generated for release mode. It is
>> need to clean them to rebuild source files for Python 2 bindings.
>>
>> So I want to update subversion/bindings/swig/INSTALL so that users don't
>> fall into this pitfall. Could anyone please make this better?
>>
>> [[[
>> * subversion/bindings/swig/INSTALL
>> (Step 2: Build and Install Subversion.): Add description for optional
>> process needed to build Python 2 bindings.
>>
>> Index: subversion/bindings/swig/INSTALL
>> ===================================================================
>> --- subversion/bindings/swig/INSTALL (revision 1877407)
>> +++ subversion/bindings/swig/INSTALL (working copy)
>> @@ -141,6 +141,10 @@
>>
>> See Subversion's own INSTALL file for details.
>>
>> + If you are using Subversion distribution tarball and want to build
>> + Python bindings for Python 2, you should run 'sh autogen.sh' before run
>> + ./configure script to rebuild build environment as non-release mode.
>
> I suggest to add this information to trunk/INSTALL, item (12), instead.
Ah, that's good. I felt somewhat uncomfortable placing this text here.
> A few tweaks to the text:
>
> + If you are using a Subversion distribution tarball and want to build
> + the Python bindings for Python 2, you should run 'sh autogen.sh' before running
> + the ./configure script, to rebuild the build environment in non-release mode.
>
> Personally, I prefer to put descriptions before instructions (i.e.,
> "you should rebuild the build environment ... by running 'sh
> autogen.sh' ..."), but this is by and large a question of style.
Agree. Actually I want to know why to do so at first in such case.
>> Make sure that Subversion's ./configure script sees your installed SWIG!
>> It tries to detect SWIG near the very end of its output.
>
> Preëxisting text, I know, but still: how about changing this to
> recommend, say, «grep '^SWIG' config.log», or some other appropriate
> command?
Then, how about this?
[[[
* INSTALL (I.C.13): Add Note that non-release mode is requires to build
SWIG Python 2 binding.
* subversion/bindings/swig/INSTALL
(BUILDING SWIG BINDINGS FOR SVN ON UNIX, Step 2): Add description how to
confirm that the ./configure could find SWIG path correctly.
Index: INSTALL
===================================================================
--- INSTALL (revision 1877936)
+++ INSTALL (working copy)
@@ -506,7 +506,13 @@
reached end of life. All users are strongly encouraged to move
to Python 3.
+ Note: If you are using a Subversion distribution tarball and want
+ to build the Python bindings for Python 2, you should rebuild
+ the build environment in non-release mode by running
+ 'sh autogen.sh' before running the ./configure script; see
+ section II.B for more about autogen.sh.
+
13. Perl 5.8 or newer (Windows only) (OPTIONAL)
To build Subversion under any of the MS Windows platforms, you
Index: subversion/bindings/swig/INSTALL
===================================================================
--- subversion/bindings/swig/INSTALL (revision 1877936)
+++ subversion/bindings/swig/INSTALL (working copy)
@@ -143,6 +143,7 @@
Make sure that Subversion's ./configure script sees your installed SWIG!
It tries to detect SWIG near the very end of its output.
+ You can find it by running 'grep "^SWIG=" config.log'.
Also make sure that the configure script sees the paths to the perl and/or
python executable you used to configure SWIG as above. If it does not then
]]]
Thanks,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] Update swig INSTALL document for Python 2 bindings
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yasuhito FUTATSUKI wrote on Thu, 14 May 2020 19:51 +0900:
> Hi,
>
> I overlooked that 'make clean-swig-py' doesn't remove SWIG generated
> source files if build-output.mk is generated for release mode. It is
> need to clean them to rebuild source files for Python 2 bindings.
>
> So I want to update subversion/bindings/swig/INSTALL so that users don't
> fall into this pitfall. Could anyone please make this better?
>
> [[[
> * subversion/bindings/swig/INSTALL
> (Step 2: Build and Install Subversion.): Add description for optional
> process needed to build Python 2 bindings.
>
> Index: subversion/bindings/swig/INSTALL
> ===================================================================
> --- subversion/bindings/swig/INSTALL (revision 1877407)
> +++ subversion/bindings/swig/INSTALL (working copy)
> @@ -141,6 +141,10 @@
>
> See Subversion's own INSTALL file for details.
>
> + If you are using Subversion distribution tarball and want to build
> + Python bindings for Python 2, you should run 'sh autogen.sh' before run
> + ./configure script to rebuild build environment as non-release mode.
I suggest to add this information to trunk/INSTALL, item (12), instead.
A few tweaks to the text:
+ If you are using a Subversion distribution tarball and want to build
+ the Python bindings for Python 2, you should run 'sh autogen.sh' before running
+ the ./configure script, to rebuild the build environment in non-release mode.
Personally, I prefer to put descriptions before instructions (i.e.,
"you should rebuild the build environment ... by running 'sh
autogen.sh' ..."), but this is by and large a question of style.
> Make sure that Subversion's ./configure script sees your installed SWIG!
> It tries to detect SWIG near the very end of its output.
Preëxisting text, I know, but still: how about changing this to
recommend, say, «grep '^SWIG' config.log», or some other appropriate
command?
Cheers,
Daniel
P.S. Speaking of Python 2, section E.1 of trunk/INSTALL says the Windows
build requires Python 2.7 or greater. Shouldn't that be updated to
require Python 3 (in 1.14 too, as the release notes already document
that)?