indygreg / python-build-standalone

Produce redistributable builds of Python
BSD 3-Clause "New" or "Revised" License
1.71k stars 107 forks source link

unix: enable tzset when x-compiling to linux target #195

Closed bcmyers closed 5 months ago

bcmyers commented 7 months ago

When cross-compiling, configure cannot detect if the target system has a working tzset function in C. This influences whether or not the compiled python will end up with the time.tzset function or not, which is used by django and other important python projects. It would be nice to have access to time.tzset on aarch64-unknown-linux-gnu.

Since all linux targets should have a working tzset function in C, we should be able to just manually indicate this to the configure script, which is what this PR does.

The motivation for this PR is as follows: Our organization uses the pre-built python interpreters from this project via rules_python and bazel. We are currently migrating several django applications to aarch64-unknown-linux-gnu to take advantage of cheaper graviton instances on AWS and hit this issue. Accepting this PR would allow us to use this project's pre-built released interpreters instead of having to maintain our own fork and build our own interpreters.

Fixes: #196

bcmyers commented 7 months ago

No worries on the timing! We've already forked your repository and, with that fork, are already building and using an aarch64-unknown-linux-gnu interpreter internally that works for us. It would be nice to eventually be able to go back to using the released interpreters from your project, but there's no rush on that. Happy to work with your schedule. After all, we use your code for free! I really believe that you don't owe anyone anything, especially when it comes to timing! :). Thank you for all the work you do!

With regards to your question about why the configure script is not doing what I think we want when cross-compiling...

Here is the code in question: https://github.com/python/cpython/blob/v3.8.18/configure#L15805. I've chosen cpython 3.8.18 at random, but the code is the same for 3.9, 3.10, 3.11 and 3.12. I also spot checked cpython 2.5.5 (released in 2010) and the code is there as well. It's been around for quite a long time it seems.

As you can see from the code, when building "normally" (i.e. when not cross-compiling), the configure script will compile a short test C program and run it on the host. It uses this short test C program to determine if it should include the time.tzset in the standard library or not. If the short test program exits 0, it compiles in that functionality during the make step. If the short test C program exits non-zero, it does not.

When cross-compiling, however, the configure script is not able to run the small test C program (for obvious reasons since it can't run an executable on the target architecture). So instead of doing this check, it just unconditionally sets ac_cv_working_tzset to no in order to be super safe.

Apparently there are some extremely old versions of linux (the comment mentions Red Hat 6.2, which was released in 2011) that do not, in fact, contain the system libraries, etc. needed in order for time.tzset to work properly in the standard library.

That said, it is highly unlikely that any version of linux you wish to support does not contain the functionality needed to allow the time.tzset function to be included in the standard library. It's just that when cross-compiling, the build system has no way to officially verify this assumption; so, again, it plays it super safe by setting ac_cv_working_tzset to no unconditionally.

Even though I don't know how old the linux distributions have to be in order to get this test C program to fail, I did check that the test C program compiles and exits 0 on the oldest system you use for non-cross-compiling builds. I couldn't check aarch64-unknown-linux-gnu on debian 8 because debian 8 apparently does not have a release for this architecture, but I was able to check that armv7l-unknown-linux-gnueabihf on debian 8 does indeed successfully compile the test C program and that it exits 0.

We could do further investigations of other old linux distributions if you'd like, but I think all of the above means that for the official debian 8 that is the oldest system you are trying to support, the configure script sets ac_cv_working_tzset to yes. The only scenario in which it doesn't is the cross-compiling scenario where cpython is being super safe and unconditionally setting the value to no, no matter what target you are building for.

So, to summarize, this PR would indeed "do the wrong thing" if you were trying to build for some linux versions released around the time of Red Hat 6.2 (2011), but if we're only looking to support debian 8 and newer than accepting this PR should (I think) only make the "cross-compiled" version do the same thing that the "non-cross-compiled" version would do if you had access to build machines that could do native (i.e. non-cross-compiled) builds.

All that said, an alternative to this PR to consider might be to just work on setting up the CI/CD system to do non-cross-compiling builds for aarch64-unknown-linux-gnu. That would also produce an interpreter for aarch64-unknown-linux-gnu that includes the time.tzset function in the standard library. Setting up CI/CD in order to be able to do native builds for aarch64-unknown-linux-gnu might be a lot more work, however, and I'm not sure you want the project to go in this direction at this time. It's worth mentioning as a potential alternative though.

bcmyers commented 7 months ago

Oh, also, it looks like CI is failing for reasons unrelated to this PR?

indygreg commented 7 months ago

CI failures look like a potential BuildKit/Docker bug not honoring ignore_errors=true. Sadness.

Try commenting out the cache-to configs at https://github.com/indygreg/python-build-standalone/blob/74f570e0ed95dade0cf1d6143c4f0868d01d8868/.github/workflows/linux.yml#L99 to unwedge CI.

bcmyers commented 7 months ago

Sounds good. I added an "unwedge CI" commit.