Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change mkfifoat and mknodat to be disabled unconditionally #161

Closed
wants to merge 1 commit into from

Conversation

samkellett
Copy link

These two functions are currently disabled if using Python 3.8 or earlier, however even if you try to link the libpython3.10.a archive you get linker errors that these two symbols don't exist because the minimum deployment version was 11.0 and these don't exist until SDK 13.0.

@samkellett samkellett changed the title Change mkfifoat and mknodat to be disabled uncondtionally Change mkfifoat and mknodat to be disabled unconditionally Feb 20, 2023
@indygreg
Copy link
Owner

Just for my understanding, what are the circumstances where you get a linker error? Are you using the official builds of this project or are you building locally?

(I have a theory as to what's going on but I want to confirm it.)

@samkellett
Copy link
Author

Sorry about the radio silence, this ended up not being an issue due to other reasons and I forgot to follow up on it.

I was building my own package but I don't think you need to to experience the bug.

If you use the python3.10 package on a project that has a required deployment version (not minimum) of 11 then I think it should break because those two symbols don't exist until 13 despite the fact that this library should work from 11 onwards.

@ZhongRuoyu
Copy link

ZhongRuoyu commented Sep 4, 2023

Hi @indygreg 👋, Homebrew maintainer here.

what are the circumstances where you get a linker error?

A circumstance would be to run pyoxidizer 0.24.0 on arm64 macOS before Ventura:

pyoxidizer init-rust-project hello_world
cd hello_world
pyoxidizer build

The above is the test we run on our pyoxidizer builds. While packaging pyoxidizer 0.24.0, we encountered the following error while running the test on arm64 macOS 11 (Big Sur) and 12 (Ventura):

    = note: ld: warning: object file (/private/tmp/pyoxidizerCTvSoP/build/target/aarch64-apple-darwin/debug/deps/libpyo3_ffi-b9ea064ef657ba4e.rlib(c72c56aa804e03f2-config.o)) was built for newer macOS version (12.0) than being linked (11.0)
            Undefined symbols for architecture arm64:
              "_mkfifoat", referenced from:
                  _os_mkfifo_impl in libpyo3_ffi-b9ea064ef657ba4e.rlib(posixmodule.o)
              "_mknodat", referenced from:
                  _os_mknod_impl in libpyo3_ffi-b9ea064ef657ba4e.rlib(posixmodule.o)
            ld: symbol(s) not found for architecture arm64
            clang: error: linker command failed with exit code 1 (use -v to see invocation)
            
  
  error: could not compile `python-oxidized-importer` due to previous error
  warning: build failed, waiting for other jobs to finish...
  error[PYOXIDIZER_PYTHON_EXECUTABLE]: adding PythonExecutable to FileManifest
      
      Caused by:
          0: building Python executable
          1: building executable with Rust project
          2: cargo build failed
         --> ./pyoxidizer.bzl:283:5
          |
      283 |     files.add_python_resource(".", exe)
          |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PythonExecutable.to_file_manifest()

(CI logs available here.)

On arm64 macOS, pyoxidizer 0.24.0 uses the PGO Python builds from this project (build 20221220, Python version 3.10) by default. They are built for macOS 11.0 with the 13.1 SDK (where mkfifoat and mknodat are present):

$ curl -fsL https://github.com/indygreg/python-build-standalone/releases/download/20221220/cpython-3.10.9+20221220-aarch64-apple-darwin-pgo-full.tar.zst | tar --use-compress-program=`which unzstd` -x
$ head python/PYTHON.json                                          
{
    "apple_sdk_canonical_name": "macosx13.1",
    "apple_sdk_deployment_target": "11.0",
    "apple_sdk_platform": "macosx",
    "apple_sdk_version": "13.1",
    "build_info": {
        "core": {
            "links": [
                {
                    "name": "dl",
$ grep -E 'HAVE_MK(FIFOAT|NODAT)' python/PYTHON.json
        "HAVE_MKFIFOAT": "1",
        "HAVE_MKNODAT": "1",
$ nm python/install/lib/libpython3.10.dylib | grep -E 'mk(fifoat|nodat)'
                 U _mkfifoat
                 U _mknodat
00000000002b1a28 t _probe_mkfifoat
00000000002b1a54 t _probe_mknodat

Currently we're working around this failure in Homebrew/homebrew-core#136910 by forcing Python 3.8 in tests run on the affected platforms. It would be great if you could handle this, considering how this may affect users on older macOS.

Thanks!

@indygreg
Copy link
Owner

As part of upgrading the GitHub Actions runners to compile aarch64-apple-darwin natively from ARM runners, the Mach-O binary validation in this project is tripping over _mkfifoat and _mknodat because they aren't weakly linked. This is happening in the 3.10+ but not 3.8 or 3.9 builds. And only the builds using LTO.

This behavior is perplexing. What's stranger is that the release builds I'm performing from a similar M1 Apple machine don't seem to reproduce the behavior!

There's a good chance I take this PR and disable mkfifoat and mknodat unconditionally. But I really want to figure out why these symbols aren't getting weakly linked in some CPython versions and build optimization levels.

@indygreg
Copy link
Owner

This change in isolation is not sufficient because additional symbols are affected when upgrading the MacOS SDK version. I've created issue #216 to track the more general issue. Closing this PR in favor of a more generic solution (to be tracked in #216).

@indygreg indygreg closed this Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants