FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-109054: Don't use libatomic on cross-compilation by vstinner · Pull Request #109211 · python/cpython · GitHub

/ cpython Public

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

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:libatomic_cross_compiler
Sep 10, 2023
ConversationCommits (1)ChecksFiles changed
Merged

gh-109054: Don't use libatomic on cross-compilation#109211
vstinner merged 1 commit into
python:mainfrom
vstinner:libatomic_cross_compiler

Conversation

vstinner commented Sep 10, 2023 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

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" (...)

bedevere-bot mentioned this pull request Sep 10, 2023
Closed
vstinner mentioned this pull request Sep 10, 2023
Merged

vstinner commented Sep 10, 2023

Copy link
Copy Markdown
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 commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

This PR should fix the 3 wasm32-emscripten buildbots. See:

vstinner mentioned this pull request Sep 10, 2023
Merged

vstinner commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

!buildbot wasm32-emscripten

bedevere-bot commented Sep 10, 2023

Copy link
Copy Markdown

🤖 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 commented Sep 10, 2023

Copy link
Copy Markdown
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 commented Sep 10, 2023

Copy link
Copy Markdown
Contributor

Thanks @vstinner!

vstinner commented Sep 10, 2023

Copy link
Copy Markdown
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.

vstinner commented Sep 10, 2023
View reviewed changes
Comment thread configure.ac Outdated
pythongh-109054: Don't use libatomic on cross-compilation
0a69b27 
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 force-pushed the libatomic_cross_compiler branch from bed957b to 0a69b27 Compare September 10, 2023 10:23

vstinner commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

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

vstinner merged commit 71b6e26 into python:main Sep 10, 2023
vstinner deleted the libatomic_cross_compiler branch September 10, 2023 16:21

erlend-aasland commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

LGTM!

erlend-aasland reviewed Sep 11, 2023
View reviewed changes
Comment thread configure.ac
]])],
[ac_cv_libatomic_needed=no], dnl build succeeded
[ac_cv_libatomic_needed=yes], dnl build failed
[ac_cv_libatomic_needed=no]) dnl cross compilation

erlend-aasland Sep 11, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

vstinner Sep 11, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh, I didn't know this value.

erlend-aasland Sep 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

erlend-aasland Sep 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh, my suggestion is made moot by #109344

colesbury mentioned this pull request Oct 10, 2024
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

erlend-aasland erlend-aasland left review comments
corona10 Awaiting requested review from corona10 corona10 is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL