build_rust: remove linker handling that broke cross compilation

[ahesford]

The changes in #139 that attempt to fix issues in #138 break cross compilation of the Void Linux python3-cryptography package. (It probably would break any cross-compiled Python package that uses setuptools-rust, but python3-cryptography is the only package in Void that uses it.) The Void package build system, xbps-src, sets up cross-compilation environments for several officially supported architectures and even more for which we do not provide prebuilt packages.

The changes in #139 are the incorrect solution to a problem that was caused by crossenv itself. The crossenv package appears to hack up the Python sysconfig file to reflect the cross-compilation environment. It then relies on the modified sysconfig data to determine how it should behave; while that might be acceptable in the package that creates the modified sysconfig, copying and relying on that behavior in setuptools-rust is wrong for several reasons:

• It enforces specific assumptions about the cross-compilation environment that are not guaranteed by anything but crossenv
• It relies on HOST_GNU_TYPE and BUILD_GNU_TYPE variables in Python sysconfig, which
  1. Appear to be largely undocumented (I can't seem to find authoritative descriptions of their purpose)
  2. Probably only exist in GNU setups, but their absence here leads to the potentially incorrect assumption that cross compilation is not taking place
  3. May describe the build-time configuration for the original Python interpreter rather than the anticipated environment in which packages using setuptools-rust are to be built. (For example, Void's Python packages for aarch64{,-musl} and armv{6,7}l{,-musl} all have sysconfig files that define these variables because the Python interpreter was cross compiled; however, that cross-compiled Python interpreter may be used for native builds of Python packages that use setuptools-rust, setuptools-rust will always assume that cross compilation is expected.)
• Attempting to pull the linker as argv[0] from $BLDSHARED and force its use in Rust builds will fail catastrophically if, e.g., $BLDSHARED is a wrapper that relies on proper ordering of all following arguments.
• Even if argv[0] is the right linker, the current logic indiscriminately throws away the next argument but preserves all following arguments as linker_args, all of which are then ignored when actually building.
• The value of PYO3_CROSS_LIB_DIR is forced, potentially masking desired variables set in the user's environment with an incorrectly determined path.

The responsibility for correctly setting up the cross-compilation environment should lie in the system that constructs the environment, not fragile logic in packages like setuptools-rust. This PR removes most of the special-case handling imposed by #139 to restore cross-compilation support in general environments. The right fix for crossenv is to properly configure its build environment:

• The environment or the user can set PYO3_CROSS_LIB_DIR and PYO3_CROSS_INCLUDE_DIR as necessary to configure the proper paths for compilation.
• CARGO_BUILD_TARGET can be set to the expected target triple. (This is already recognized in the build_rust class.)
• The CARGO_TARGET_<triple>_LINKER environment variable can be set to the proper linker for cross-compilation.
• If the environment really needs additional customization, there is already preservation of RUSTFLAGS in setuptools-rust.

This implies that crossenv must do a little extra work by, e.g., modifying the virtualenv activation script to set these variables (or providing its own activation script to do so before calling the stock activation script). However, doing this work is the right fix for this problem, not special-casing stuff in setuptools-rust.

cc: @benfogle (original author) @davidhewitt (did the merge)

[kanavin]

I confirm that there is the same issue in Yocto Linux, and it's fixed by this PR - thanks!

[benfogle]

Well, as the guy who wrote it, I don't think it's quite that bad 😄! I'll leave it to the maintainers to make a decision on this PR, but I can provide some clarification here.

First, I'm very interested in all things related to Python cross-compiling, and I would love to see what exactly the issue was that you were facing. Can you provide a link to the problems you have been seeing? (I'm especially interested in Yocto, because that's my day job.)

It enforces specific assumptions about the cross-compilation environment that are not guaranteed by anything but crossenv

None of the code here is crossenv specific. It should be valid for any system using the _PYTHON_SYSCONFIGDATA_NAME method of cross-compiling, which, as you said, "hacks up sysconfig." This is the closest thing to a standard method of cross-compling that Python has, and it is used by crossenv, Yocto, Buildroot, and Void.

It relies on HOST_GNU_TYPE and BUILD_GNU_TYPE variables in Python sysconfig

Well, they're arguably more official than _PYTHON_SYSCONFIGDATA_NAME, which is an implementation detail that caught on. A little more information about them: These are the parameters passed to --build and --host at build time when cross compiling the interpreter. For native builds, they have the same (autodetected) value. They will be present on any system that uses automake to generate the makefile, which ought to be any Posix system. If they're not there, then, we may indeed fail to recognize that we're cross compiling. That was the very issue that motivated #139 in the first place, so the fallback is no worse than it was before. Non-posix architectures will need to be handled in future PRs, but in the mean time, the PYO3_... environment variables are available.

that cross-compiled Python interpreter may be used for native builds of Python packages

Could you elaborate? This doesn't seem like it would ever work.

Attempting to pull the linker as argv[0] from $BLDSHARED...

For context: The reason for this is that a lot of distros like to set it to something like gcc -pthreads. I agree it's not a perfect default.

current logic indiscriminately throws away the next argument

I believe you are confusing str.partition with str.split.

The value of PYO3_CROSS_LIB_DIR is forced

This logic is only meant to provide defaults in case the PYO3_... variables aren't defined. If it's somehow overriding them, that's definitely a bug, and I can see how that would cause issues. (Are you perhaps misunderstanding dict.setdefault?)

The responsibility for correctly setting up the cross-compilation environment should lie in the system that constructs the environment

I 100% agree, but we're coming at this from very different perspectives. Crossenv was originally conceived as a tool for developers with some self-contained app that they wanted to distribute on a different architecture. Crossenv's job is to cross compile the app along with any dependencies it may have, with minimum pain for the developer. If one dependency uses setuptools-foo and its dependency uses setuptools-bar, and they both have different ideas of how to pass along cross-compiling information, then to build, we end up with an ever-growing list of special cases that either the developer or crossenv has to handle. This doesn't really bother a package distro maintainer, since they've already committed to individually packaging some subset of PyPI, but it's a real headache for someone who just wants to distribute a single app.

In that light, I believe it's entirely appropriate to attempt to use the most standard-ish API available to handle the common cases. If additional customization is needed, then the developer can set the PYO3_... variables or RUSTFLAGS.

Again, I'd really like to help you diagnose whatever issues you are running into with this code, but I suspect the ultimate fix might be a little more measured that what you have here.

[ahesford]

My central position is that setuptools-foo and setuptools-bar (or, in this specific instance, setuptools-rust) should not have "different ideas of how to pass along cross-compiling information" because none of these packages should have any idea how to pass along cross-compiling information. The problems introduced in #139 were caused expressly because setuptools-rust was given responsibility for guessing the right linker when it detects cross-compilation. But setuptools-rust shouldn't care about the linker! It should be crossenv, not the multitude of Python packages, that knows how to configure the compilers for the various native and cross builds that take place in its purview. The crossenv environment should be pre-configured so that compiler and linker invocations like $CC, $CXX, $LD, rustc --target <blah>, etc., spit out the right object code and executables for the build platform. crossenv should be setting these environment variables. For rustc, you can write a cargo config or you can use variables like the above-noted CARGO_TARGET_<triple>_LINKER and CARGO_BUILD_TARGET.

If crossenv isn't configuring the environment to properly generate objects and executables for the build system, it isn't preparing a functional cross-compilation environment. Forcing this responsibility into setuptools-rust (or other Python distribution mechanisms) breaks the very abstractions that seem to be the entire raison d'être of crossenv. If setuptools-rust has to know how to drive the cross-compilation, why use crossenv at all?

Specific points are addressed below.

First, I'm very interested in all things related to Python cross-compiling, and I would love to see what exactly the issue was that you were facing. Can you provide a link to the problems you have been seeing? (I'm especially interested in Yocto, because that's my day job.)

Void cross-builds of python3-cryptography failed because argv[0] of $BLDSHARED is just gcc but, in the cross-compilation environment of xbps-src, gcc is the native linker. Thus, setuptools-rust correctly builds a bunch of, e.g., aarch64 objects using rustc with a proper --target, but the changes in #139 cause the insertion of -Clinker=gcc that cause the linker to fail. (I don't have the build logs handy; I encountered this error on July 6 and reverted python3-setuptools-rust immediately pending further investigation, and it's difficult to find logs from our buildbot front-end so far back.)

None of the code here is crossenv specific. It should be valid for any system using the _PYTHON_SYSCONFIGDATA_NAME method of cross-compiling, which, as you said, "hacks up sysconfig." This is the closest thing to a standard method of cross-compling that Python has, and it is used by crossenv, Yocto, Buildroot, and Void.

I am well aware of how Void handles sysconfig data for cross-compilation of Python packages. I was under the impression that crossenv was actually rewriting the sysconfig file when setting up an environment (*e.g., by modifying the path of argv[0] in $BLDSHARED). This may or may not be true. If it is, then I strongly object to carrying the assumptions about your modifications into setuptools-rust. If not, there's nothing wrong with relying on overriding _PYTHON_SYSCONFIG_DATA_NAME, but your particular use to grab the linker is problematic.

Overridding the sysconfig path is the right way to gather basic configuration information for the build target when cross-compiling, such as sizes of basic types, extensions for compiled extensions and the like. However, it is not suitable for gleaning compiler and linker settings for at least two reasons: the first, which I noted before, is that $BLDSHARED might actually refer to a wrapper that needs all its arguments, in the prescribed order, to function; the second is that the sysconfig file that is referenced might have been generated for a natively compiled Pyton interpreter (or a cross-compiled Python interpreter with a sysconfig file that has been adapted to do the right thing when the interpreter is installed and run on the build system). In either case, attempting to glean the linker (actually the compiler and linker, since I believe $BLDSHARED is supposed to be usable as the leading arguments of a command that takes a bunch of C source files and spits out a shared object representing a CPython extension).

It relies on HOST_GNU_TYPE and BUILD_GNU_TYPE variables in Python sysconfig

Well, they're arguably more official than _PYTHON_SYSCONFIGDATA_NAME, which is an implementation detail that caught on. A little more information about them: These are the parameters passed to --build and --host at build time when cross compiling the interpreter. For native builds, they have the same (autodetected) value. They will be present on any system that uses automake to generate the makefile, which ought to be any Posix system. If they're not there, then, we may indeed fail to recognize that we're cross compiling. That was the very issue that motivated #139 in the first place, so the fallback is no worse than it was before. Non-posix architectures will need to be handled in future PRs, but in the mean time, the PYO3_... environment variables are available.

Relying on HOST_GNU_TYPE and BUILD_GNU_TYPE may be problematic not only if they are missing, but sometimes even when they are present. (See next point.)

that cross-compiled Python interpreter may be used for native builds of Python packages

Could you elaborate? This doesn't seem like it would ever work.

When I say "native builds", I mean native on the build system, not the host system. Void cross-compiles Python for {aarch64,armv{6,7}l}{,-musl} because we don't have native ARM builders. Hence, the sysconfig for the Python interpreter (the python3 package itself) includes descriptors like HOST_GNU_TYPE=aarch64-unknown-linux-gnu and BUILD_GNU_TYPE=x86_64-unknown-linux-gnu (for glibc; musl may or may not have a slightly different form). (NB: we do actually modify paths in the sysconfig file during packaging of python and python3; at build time, the cross triplet can make its way into paths because we put tooling for the build system in /usr/<triple> and even provide some wrappers in a temporary build directory. Because the sysconfig must be accurate when the cross-compiled interpreter is actually run on the intended architecture, these cross-specific references must be replaced with "ordinary" system locations.)

If an aarch64 user installs the python3 package and then attempts to natively build a project using setuptools-rust, the changes from #139 will lead setuptools-rust (and crossenv) to incorrectly deduce that the package is being cross compiled. The {HOST,BUILD}_GNU_TYPE variables describe how the Python interpreter itself was compiled, not necessarily how subsequent Python packages should be compiled.

Again, my view is that setuptools-rust should never care at this level of detail how to build and link rust programs. After passing --target <cross-triple> to rustc, the rust compiler itself should know how to pick a linker. (Because it doesn't we tell it with environment variables.)

Attempting to pull the linker as argv[0] from $BLDSHARED...

For context: The reason for this is that a lot of distros like to set it to something like gcc -pthreads. I agree it's not a perfect default.

It's not just an imperfect default, it provides no mechanism for overrides. If the setuptools-rust maintainers reject my assertion that this project should never care about the linker because that's the job of whoever is setting up the environment, the linker determination logic should at least be gated behind some environment variable that can control it.

As it stands, without killing the linker selection from #139, I can forcefully override the linker by adding the right one back with -Clinker=<right-linker> to RUSTFLAGS, which works (by accident) because RUSTFLAGS are appended instead of prepended to the other arguments. However, this means setuptools-rust goes through a dance to pick a linker, adds its own -Clinker argument, and then we have to compete with an additional -Clinker argument to correct the mistake. And what happens in the future should rustc decide that RUSTFLAGS should be processed before any command-line arguments or that multiple -Clinker arguments cause a compiler error rather than a last-out wins?

current logic indiscriminately throws away the next argument

I believe you are confusing str.partition with str.split.

Mea culpa.

The value of PYO3_CROSS_LIB_DIR is forced

This logic is only meant to provide defaults in case the PYO3_... variables aren't defined. If it's somehow overriding them, that's definitely a bug, and I can see how that would cause issues. (Are you perhaps misunderstanding dict.setdefault?)

Another mea culpa.

[messense]

I'm ok with merging this since it did not break test-crossenv in CI.

[kanavin]

First, I'm very interested in all things related to Python cross-compiling, and I would love to see what exactly the issue was that you were facing. Can you provide a link to the problems you have been seeing? (I'm especially interested in Yocto, because that's my day job.)

Yocto log with the failure attached. The failure disappears if I add the changes in this pull request.
log.do_compile.790575.txt

[davidhewitt]

Thanks @ahesford @kanavin for the discussion and evidence. Sorry it's taken me a few days to have some time to sit down and read this all. I get the sense that this logic has spent you a great deal of time and frustration to investigate as well as impacted your users, and I'm sorry for that.

As in #139 (comment), my personal experience is still heavily weighted towards Python/Rust language integration rather than cross-compiling the languages in tandem. Input from more knowledgeable parties is extremely valued to make sure that setuptools-rust provides the widest compatibility possible.

I agree with @benfogle that this is a case where different needs and toolchains of individual developers and package maintainers make the space complicated. The impression I have is that cross-compiling Python packages is very much DIY / best-effort. I would love to see this improved; I don't have time to champion such an effort myself though would gladly make setuptools-rust compliant with any official standard. @ahesford @kanavin I would strongly encourage you to join the thread at https://discuss.python.org/t/towards-standardizing-cross-compiling/10357/25, where @benfogle has been floating ideas for a possible future PEP.

---

Circling back to this PR, on balance I agree with @messense that CI demonstrates this is mergeable as-is and setuptools-rust would still be compatible with crossenv. However, I think it removes more functionality than it strictly needs to, so I would like us to discuss on a few of the changes before we commit. See comments below. Hopefully we can reach an implementation which suits all parties.

[davidhewitt]

As per discussion here I am now 100% in agreement that this was wrong and it is correct to remove linker handling from setuptools-rust.

[davidhewitt]

Correction: this does not "force" PYO3_CROSS_LIB_DIR - setdefault will not overwrite any value already in the environment. So build environments should be untainted by this.

Whether setuptools-rust should be attempting to set this is a separate question. I think it's reasonable for it to make an educated guess if it's on balance helpful for users. As this value comes from the sysconfig, it seems reasonable to me to use that as a first guess.

[davidhewitt]

With an appropriate implementation of setuptools_rust that handles all build environment correctly, I still think there's value in a best-effort attempt to recognise the correct rust build target for use-cases like crossenv. My preference would be to not completely remove this, however the heavy-handedness with which it splats the build environment absolutely needs fixing.

[ahesford]

I've been thinking about the right implementation for _detect_unix_cross_compile_info, but don't have any good ideas right now. I'm also not terribly familiar with rust, but wound up with responsibility for setuptools-rust packaging in Void because it is required by our python3-adblock package as well as python3-cryptography.

For the reasons I've noted earlier, I think looking for *_GNU_TYPE variables in the target sysinfo is wrong because it will reflect the cross or native build environment of the Python interpreter rather than the compilation intent for the package being built with setuptools-rust. The best method here would be determining the native triple for the rust compiler and comparing that to self.target, but I don't know how to determine a native triple short of hacks like looking at uname output.

[kanavin]

For the reasons I've noted earlier, I think looking for *_GNU_TYPE variables in the target sysinfo is wrong because it will reflect the cross or native build environment of the Python interpreter rather than the compilation intent for the package being built with setuptools-rust. The best method here would be determining the native triple for the rust compiler and comparing that to self.target, but I don't know how to determine a native triple short of hacks like looking at uname output.

That's what GNU autotools do though (look at uname).

However, Yocto project does not generally trust any guessing that the components might try to do: we prefer to pass in all three triplets (host/build/target) explicitly via pre-written config files or command line switches.

[kanavin]

Specifically, here:
https://git.savannah.gnu.org/cgit/config.git/tree/config.guess

[ahesford]

Yeah, I think the better check for cross-compilation in this project is just "is self.target (read from CARGO_BUILD_TARGET) defined?"

Aside from checking the *_GNU_TYPE macros to pick cross info, all the _detect_unix_cross_compile_info does is attempt to determine the cross_lib path and set a default PYO3_CROSS_LIB_DIR based on it. I'd be OK with preserving that logic. (It determines the wrong value in our Void setup, but because I explicitly set the environment variable to the right value beforehand, it has no ill effect.)

[davidhewitt]

On reflection this line is clearly wrong - should have just been return _TargetInfo(forced_target_info)and not carry any of the detected cross-compile with it. (The user is already setting --target externally, they should be totally able to set the rest of the flags in this case.)

[jameshilliard]

FYI getting the linker settings right for cargo cross compilation is rather complex. Our infrastructure passes our cargo cross environment in addition to our python cross environment and some pyo3 specific variables which generally seems to work for packages like cryptography.

[davidhewitt]

Thanks all for the discussion here. I've come to opinion that while it's potentially useful for setuptools-rust to make an educated guess at cross-compile configuration, there's some downsides to the current implementation:

• it is unlikely to be correct in a lot of situations
• there's no way to override the incorrect guess (which we've all agreed above is problematic)
• where it is correct (crossenv), crossenv should be able to use other mechanisms to set up a working build environment anyway

So I think the simplest course of action is to merge this PR as-is, and then if anyone comes forward in future with better heuristics for setuptools-rust to consider we can evaluate those later.

I'll rebase and push this through. Thanks.