classification
Title: swap distutils build_ext and build_py commands to allow proper SWIG extension installation
Type: behavior Stage: patch review
Components: Distutils Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: dstufft, eric.araujo, jdemeyer, jlvandenhout
Priority: normal Keywords: patch

Created on 2019-06-12 12:11 by jlvandenhout, last changed 2019-06-13 10:33 by jlvandenhout.

Pull Requests
URL Status Linked Edit
PR 14014 open python-dev, 2019-06-12 12:20
Messages (6)
msg345330 - (view) Author: Jeroen van den Hout (jlvandenhout) * Date: 2019-06-12 12:11
Currently when building an extension which lists a SWIG interface (.i) file as a source file, SWIG creates the files correctly in the build_ext command. Unfortunately this command is run after the build_py command, so the python files (.py) created in the build_ext command are never copied to the installation.
To fix this, swap build_ext and build_py commands in the sub_commands attribute of the build command.
msg345333 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-06-12 12:36
> SWIG creates the files correctly in the build_ext command

One could argue that running SWIG should be an entirely different step, not part of build_ext.

I'm afraid that your idea of swapping build_ext and build_py will be rejected because it has a high chance of breaking stuff: we have always done build_py before build_ext and packages probably rely on this order.
msg345408 - (view) Author: Jeroen van den Hout (jlvandenhout) * Date: 2019-06-12 21:09
Well, if I look at the source code build_ext explicitly calls for SWIG to handle interface files. Also, in case of commands like 'pip install <package>', the build sequence is predefined anyways, unless you explicitly define your own sequence in a custom command, in which case you are already tempering with default behavior.

That's why I argue that building and copying SWIG generated files correctly should be the default behavior.
msg345427 - (view) Author: Jeroen van den Hout (jlvandenhout) * Date: 2019-06-12 23:19
I'm sorry, I read your comment completely wrong, so please disregard my previous comment.

I would argue the sequence doesn't really matter that much for non SWIG packages, since those will install correctly no matter if the extensions are built and copied first or not. In the non-SWIG case they are truly independent commands. It is only in the SWIG packages that they become dependent on each other.

On top of that I see more and more questions on stack exchange and other forums asking why their SWIG builds fail to install correctly, implying that more and more developers are using this awesome feature and start to rely on it.
msg345460 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-06-13 07:01
> I would argue the sequence doesn't really matter that much for non SWIG packages

I don't think that this is correct. Or at least, it's not obvious to me that this is correct. It's not uncommon for projects to extend distutils in various ways and the proposed change may break those packages.

Adding a separate build step for SWIG (then the order would be build_swig, build_py, build_ext) would be safer and IMHO also more logical.
msg345495 - (view) Author: Jeroen van den Hout (jlvandenhout) * Date: 2019-06-13 10:33
> Adding a separate build step for SWIG (then the order would be build_swig, build_py, build_ext) would be safer and IMHO also more logical.

I like this suggestion and contemplated it. However SWIG interface files are defined in Extension instances, which are inherently tight to the build_ext command. Breaking this link would require introspection of extensions before deciding on which command to invoke, or a completely separate SWIG Extension class, both of which require a change in usage.

> It's not uncommon for projects to extend distutils in various ways and the proposed change may break those packages.

I argue that those packages are already tempering with default behavior of distutils, while the change I am proposing is fixing a flaw in basic functionality and might I say 'implied behavior' of distutils (as the docs say distutils understands SWIG, while it isn't currently even capable of installing correctly when SWIG files are used).

The build_ext and build_py commands are truly separate commands, except when SWIG interface files are used:

- build_py is responsible for copying pure python files to their correct directories, potentially compiling them to .pyc files.

- build_ext is responsible for compiling and linking C/C++ files and copying resultant files to their correct directories. It is only when SWIG files are supplied that a .py file is produced, which quite neatly would be picked up by build_py, given the chance.
History
Date User Action Args
2019-06-13 10:33:38jlvandenhoutsetmessages: + msg345495
2019-06-13 07:01:26jdemeyersetmessages: + msg345460
2019-06-12 23:19:19jlvandenhoutsetmessages: + msg345427
2019-06-12 21:09:19jlvandenhoutsetmessages: + msg345408
2019-06-12 12:36:15jdemeyersetnosy: + jdemeyer
messages: + msg345333
2019-06-12 12:20:54python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request13878
2019-06-12 12:11:36jlvandenhoutcreate