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

Created on 2019-06-12 12:11 by jlvandenhout, last changed 2021-02-03 18:08 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14014 closed python-dev, 2019-06-12 12:20
Messages (9)
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) * (Python triager) 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) * (Python triager) 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.
msg365156 - (view) Author: John Andersen (pdxjohnny) * Date: 2020-03-27 15:46
I'm going to take a stab at this by adding build_swig which will run if the list of files only contains .i files.
msg368322 - (view) Author: John Andersen (pdxjohnny) * Date: 2020-05-07 04:21
I haven't made much progress on the fix yet. But I have a workaround here: https://github.com/tpm2-software/tpm2-pytss/commit/9952e374b4d9b854aea12c667dd7d7ab4ad501a9
msg386267 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-02-03 18:08
Distutils is now deprecated (see PEP 632) and all tagged issues are being closed. From now until removal, only release blocking issues will be considered for distutils.

If this issue does not relate to distutils, please remove the component and reopen it. If you believe it still requires a fix, most likely the issue should be re-reported at https://github.com/pypa/setuptools
History
Date User Action Args
2021-02-03 18:08:02steve.dowersetstatus: open -> closed

nosy: + steve.dower
messages: + msg386267

resolution: out of date
stage: patch review -> resolved
2020-05-07 04:21:36pdxjohnnysetmessages: + msg368322
2020-03-27 15:46:39pdxjohnnysetmessages: + msg365156
2020-03-27 00:19:52pdxjohnnysetnosy: + pdxjohnny
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