nightlark / swig-pypi

pip install swig. SWIG Python wheel for PyPI.
Apache License 2.0
16 stars 5 forks source link

Add support for using existing system swig binary. #114

Open jameshilliard opened 10 months ago

jameshilliard commented 10 months ago

For cross compilation build environments like buildroot it would be handy to have something like an override env variable that tells swig-pypi to not actually build/install swig but to instead use an existing system swig provided by our own build infrastructure.

One reason for this is that we don't want any of our package builds to be invoking any external downloads at all as we want out build infrastructure to be handling that itself(ie so that we can build everything offline using our package download mirror infrastructure if necessary).

nightlark commented 10 months ago

Are you trying to build a Python package that lists swig under its build-system.requires section in pyproject.toml? As in:

[build-system]
requires = ["swig"]

How are you handling this issue for Python packages that list “cmake”, “ninja”, "setuptools", "setuptools-scm", “scikit-build”, or “poetry_core” as build system requirements? I don’t think this is a unique problem, so the solution to all of these should be pretty similar.

For Python packages that are using scikit-build-core as the build system backend, I believe that if scikit detects a suitable copy of swig, CMake, or ninja on the system, then it won’t install the PyPI packages for them.

Otherwise, would the pip option --no-build-isolation work? I suspect that this is what you’d want to do, since that option was intended for situations where a user wants to manage the build environment themselves.

The other alternative I see is including any packages listed under build-system requires in the package download mirror. I don’t think an environment variable would be able to do what you’d want, since pip needs to download the required build system requirements (either source or binary wheel) before any of the code that is part of swig-pypi has a chance to run.

jameshilliard commented 10 months ago

Are you trying to build a Python package that lists swig under its build-system.requires section in pyproject.toml?

Yes

How are you handling this issue for Python packages that list “cmake”, “ninja”, "setuptools", "setuptools-scm", “scikit-build”, or “poetry_core” as build system requirements?

We use our package infrastructure to install those dependencies to satisfy the pep517 build-system requirements.

For Python packages that are using scikit-build-core as the build system backend, I believe that if scikit detects a suitable copy of swig, CMake, or ninja on the system, then it won’t install the PyPI packages for them.

This doesn't appear to work, I tried making our swig-pypi package depend on our system swig package and it still builds swig from source regardless.

Otherwise, would the pip option --no-build-isolation work?

We don't use pip as pip is not cross compilation compatible. Our build infrastructure wraps build/installer instead.

The other alternative I see is including any packages listed under build-system requires in the package download mirror.

Actually this is more or less how our build infrastructure already works, all dependencies are built/installed separately by our infrastructure.

since pip needs to download the required build system requirements (either source or binary wheel) before any of the code that is part of swig-pypi has a chance to run

This doesn't make sense in our case since we don't support pip at all for python package builds and don't want random python dependencies doing their own network downloads, our package build infrastructure can download build and install system swig prior to installing swig-pypi. We need a way to tell swig-pypi not to try and do it's own dependency installation here since we want to handle that in our infrastructure separately.

nightlark commented 10 months ago

I guess I’m not seeing why you can’t rely on the same infrastructure you use to satisfy other pep517 build-system requirements. You could install system cmake, and you’d have this same problem with the cmake package on PyPI.

If you’re installing from a binary wheel, it isn’t making any random network downloads; the binary wheel you download from PyPI is self-contained. If you’re installing from the source tarball then setup.py and CMakeLists.txt is modeled after what cmake-python-distributions and ninja-python-distributions do for fetching a copy of the source code and building from source.

———

Since it sounds like your build infrastructure wrapper (is the source code available?) is already doing some inspection of the build-system.requires, you could add a special case to it which ignores “swig” as a dependency (which is essentially what sciki-build-core does when a suitable system swig version is detected). Since it sounds like you want a special case for some pep517 build system requirements and not others, the wrappers sound like the place to handle that.

It might be worth opening a PR with some proposed changes to better illustrate what you’re trying to do, since I’m not that familiar with your build infrastructure and what it is doing. One other thing that has me puzzled though is it sounds like you are already able to detect when swig needs to be installed in order to install the system copy of swig, in which case there is no reason to even bother installing swig-pypi.

If you have zoom/webex/teams we could schedule a call so you can show me what your setup looks like, to better help me understand the problem and what a suitable solution would look like.

jameshilliard commented 10 months ago

I guess I’m not seeing why you can’t rely on the same infrastructure you use to satisfy other pep517 build-system requirements.

We build python packages from sdists, our download infrastructure however expects them to contain the complete package and not a wrapper package that downloads/builds software from random external sources bypassing our package download infrastructure.

You could install system cmake, and you’d have this same problem with the cmake package on PyPI.

Well...we currently build cmake packages using our own dedicated cmake infrastructure, we haven't added support for the cmake pypi package yet so that will be something that may needs some tweaks as well.

Since it sounds like your build infrastructure wrapper (is the source code available?) is already doing some inspection of the build-system.requires

Well build enforces that the dependencies are present but we manually specify them in our build infrastructure. I have a preliminary patch for adding swig-pypi to buildroot. Most of our python build infrastructure wrapper code is here if you're curious.

you could add a special case to it which ignores “swig” as a dependency (which is essentially what sciki-build-core does when a suitable system swig version is detected)

Maybe just an env variable in swig-pypi that skips building swig and just checks that the binary is in path and wraps that or something? I'm a bit wary of messing with pep517 build dependency validation.

Since it sounds like you want a special case for some pep517 build system requirements and not others, the wrappers sound like the place to handle that.

I don't think our wrappers have that level of control.

It might be worth opening a PR with some proposed changes to better illustrate what you’re trying to do, since I’m not that familiar with your build infrastructure and what it is doing.

Yeah I can probably give that a try if you don't see a good approach.

One other thing that has me puzzled though is it sounds like you are already able to detect when swig needs to be installed in order to install the system copy of swig, in which case there is no reason to even bother installing swig-pypi.

Well...it's more manually specified, the main reason for installing swig-pypi is simply because it's an enforced pep517 build dependency.

If you have zoom/webex/teams we could schedule a call so you can show me what your setup looks like, to better help me understand the problem and what a suitable solution would look like.

Yeah, can probably do that if necessary.

I'm thinking just adding an env variable like SWIG_PYPI_USE_SYSTEM_SWIG that changes swig-pypi's setup.py logic to instead check that an appropriate swig binary is present in the path instead of building swig in setup.py may be sufficient. Maybe it should also install a different wrapper script as well that searches in the system path instead of for the vendored swig?

henryiii commented 7 months ago

If you'd like to see how to do this, see https://scikit-build.readthedocs.io/en/latest/usage.html#adding-cmake-as-building-requirement-only-if-not-installed-or-too-low-a-version. You should not sabotage the build process of a package just because you need an PEP517 build dependency. See the extensive discussions for the ninja or cmake packages. This would break the ability to use the package correctly - for example, pip install swig==4.1.0 might detect swig 3 and then not build swig (making it opt in helps, but if you are manually setting variables, you might as well build without isolation). You can't tell what a user asked for when you are building.

You can wrap the PEP 517 build hooks and only add the wrapper tool (in this case, swig) if swig is not present on the system or is too old. Or you can build without isolation and disable the check in build that verifies build dependencies are present. If you can't build, the build dependencies aren't present. :)

(I really dislike that check, and wish it was opt-in instead of opt-out, like pip).