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

bpo-41916: allow cross-compiled python to have -pthread set for CXX #22525

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

virtuald
Copy link
Contributor

@virtuald virtuald commented Oct 3, 2020

When cross-compiling, the compile/run test for -pthread always fails so -pthread will never be automatically set without an override from the cache. ac_cv_pthread can already be overridden, so do the same thing for ac_cv_cxx_thread.

I've validated that my cross-compiled python has the variables set now when configured via:

   ./configure --host=$TARGET_HOST --build=$BUILD_HOST --prefix=$PREFIX \
        --disable-ipv6 --enable-unicode=ucs4 \
        ac_cv_file__dev_ptmx=no ac_cv_file__dev_ptc=no \
        ac_cv_have_long_long_format=yes \
        ac_cv_pthread_is_default=no ac_cv_pthread=yes ac_cv_cxx_thread=yes; \

I now get as expected:

>>> import pprint, distutils.sysconfig
>>> pprint.pprint({i: distutils.sysconfig.get_config_vars(i)[0] for i in ('CC', 'CXX', 'LDSHARED')})
{'CC': 'arm-frc2020-linux-gnueabi-gcc -pthread',
 'CXX': 'arm-frc2020-linux-gnueabi-c++ -pthread',
 'LDSHARED': 'arm-frc2020-linux-gnueabi-gcc -pthread -shared'}

I haven't yet checked to see if it all works quite yet, will do later today.

https://bugs.python.org/issue41916

@virtuald
Copy link
Contributor Author

virtuald commented Oct 6, 2020

Some testing indicates that this worked without issues. Given that the tests all seem to pass on CI, I think this is good to go.

@virtuald
Copy link
Contributor Author

I have confirmed that this does fix the issues I was running into, and my cross compiled packages seem to be fine.

Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

This fix looks correct. @jaraco could you have a look?

@virtuald
Copy link
Contributor Author

virtuald commented Mar 4, 2022

I've been using it to build python 3.9.x and more recently python 3.10.x without any issues.

@FFY00
Copy link
Member

FFY00 commented Mar 4, 2022

Perhaps, just add a news item to make it quicker to merge. I think the code is good, it is just missing a news entry.

@kumaraditya303
Copy link
Contributor

You can use https://blurb-it.herokuapp.com/ to add a news fragment.

@jaraco
Copy link
Member

jaraco commented Apr 4, 2022

I'm afraid I don't have a lot of experience with this aspect of CPython. Perhaps @ambv could review or suggest a reviewer?

@virtuald
Copy link
Contributor Author

virtuald commented Apr 4, 2022

FWIW, I've been using the patch to cross-compile python for use in CI since I've made the patch.

@virtuald
Copy link
Contributor Author

Rebased on main; integrated changes that were made in #29485

@virtuald
Copy link
Contributor Author

@tiran seems to make a lot of changes to configure.ac, perhaps they might be a better reviewer for this?

When cross-compiling, the compile/run test for -pthread always fails so -pthread
will never be automatically set without an override from the cache. ac_cv_pthread
can already be overridden, so do the same thing for ac_cv_cxx_thread.
@FFY00
Copy link
Member

FFY00 commented Oct 18, 2022

@tiran, do have time to have a quick look at this PR? Thanks!

@ericsnowcurrently
Copy link
Member

IIRC, @gpshead has some interest in cross-compilation and @erlend-aasland has experience with the configure script.

@gpshead gpshead merged commit cc87487 into python:main Jan 5, 2023
@virtuald virtuald mannequin mentioned this pull request Jan 5, 2023
@virtuald virtuald deleted the allow-cxx-pthread-override branch January 6, 2023 02:36
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.

None yet

9 participants