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

gh-109054: Don't use libatomic on cross-compilation #109211

Merged
merged 1 commit into from Sep 10, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 10, 2023

configure no longer uses libatomic by default when Python is cross-compiled. The LIBATOMIC variable can be set manually in this case:

./configure LIBATOMIC="-latomic" (...)

@vstinner
Copy link
Member Author

The LIBATOMIC variable can be set manually in this case: ./configure LIBATOMIC="-latomic" (...)

Should it be documented in https://docs.python.org/dev/using/configure.html?

cc @erlend-aasland

@vstinner
Copy link
Member Author

@vstinner
Copy link
Member Author

!buildbot wasm32-emscripten

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit bed957b 🤖

The command will test the builders whose names match following regular expression: wasm32-emscripten

The builders matched are:

  • wasm32-emscripten node (dynamic linking) PR
  • wasm32-emscripten node (pthreads) PR
  • wasm32-emscripten browser (dynamic linking, no tests) PR

@vstinner
Copy link
Member Author

The "Configure host Python" step now says no for libatomic, as expected, good!

wasm32-emscripten node (dynamic linking) PR

checking whether libatomic is needed by <pyatomic.h>... no

wasm32-emscripten node (pthreads) PR

checking whether libatomic is needed by <pyatomic.h>... no

wasm32-emscripten browser (dynamic linking, no tests) PR

checking whether libatomic is needed by <pyatomic.h>... no

@hoodmane
Copy link
Contributor

Thanks @vstinner!

@vstinner
Copy link
Member Author

test_threading failed on Windows x64: it's an unrelated known bug, see: #108987 (I proposed a fix). I re-ran the Windows x64 job.

configure.ac Outdated Show resolved Hide resolved
configure no longer uses libatomic by default when Python is
cross-compiled. The LIBATOMIC variable can be set manually in this
case:

    ./configure LIBATOMIC="-latomic" (...)
@vstinner
Copy link
Member Author

I wrote PR #109224 to document LIBATOMIC and other configuration variables.

@vstinner vstinner merged commit 71b6e26 into python:main Sep 10, 2023
20 of 21 checks passed
@vstinner vstinner deleted the libatomic_cross_compiler branch September 10, 2023 16:21
@erlend-aasland
Copy link
Contributor

LGTM!

]])],
[ac_cv_libatomic_needed=no], dnl build succeeded
[ac_cv_libatomic_needed=yes], dnl build failed
[ac_cv_libatomic_needed=no]) dnl cross compilation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have used ac_cv_libatomic_needed=n/a for a more accurate result message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't know this value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't know this value.

Actually, the value is up to you; it's not a magical GNU Autoconf value :) See docs for AC_CACHE_CHECK. The signature for that macro is:

AC_CACHE_CHECK (message, cache-id, commands-to-set-it)

Quoting the docs:

It calls AC_MSG_CHECKING for message, then AC_CACHE_VAL with the cache-id and commands arguments, and AC_MSG_RESULT with cache-id.

In out case, cache-id is ac_cv_libatomic_needed. Whatever we set it to will be displayed to the user at the end of the check (via the implicit AC_MSG_RESULT call). So if we set ac_cv_libatomic_needed to n/a, the user should see this when cross-compiling:

checking whether libatomic is needed by <pyatomic.h>... n/a

Currently, the user will see this when cross-compiling:

checking whether libatomic is needed by <pyatomic.h>... no

The "no" may lead the user to incorrectly assume that configure actually checked whether libatomic was needed (and that configure concluded it was not).

I think it may be worth it to adjust this message. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my suggestion is made moot by #109344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants