GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-41916: allow cross-compiled python to have -pthread set for CXX by virtuald · Pull Request #22525 · python/cpython

Conversation

@virtuald

@virtuald virtuald commented Oct 3, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rchildre3 reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 3, 2020
@bedevere-bot bedevere-bot added the awaiting review label Oct 3, 2020
@virtuald

virtuald commented Oct 6, 2020

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@virtuald virtuald mentioned this pull request Oct 17, 2020
-pthread not set in cross-compiled python gcc flags robotpy/crossenv#36
Closed
@virtuald virtuald mentioned this pull request Oct 24, 2020
Cross-compile from sources for Yocto/Linux scipy/scipy#8571
Closed
@virtuald

virtuald commented Oct 24, 2020

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

FFY00
FFY00 approved these changes Mar 4, 2022
View reviewed changes

@FFY00 FFY00 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Mar 4, 2022
@virtuald

virtuald commented Mar 4, 2022

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FFY00

FFY00 commented Mar 4, 2022

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303

kumaraditya303 commented Mar 4, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@virtuald
virtuald force-pushed the allow-cxx-pthread-override branch from 6ff346a to 0fdc88d Compare March 4, 2022 15:48
@jaraco

jaraco commented Apr 4, 2022

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@virtuald

virtuald commented Apr 4, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@virtuald
virtuald force-pushed the allow-cxx-pthread-override branch from 0fdc88d to 77af732 Compare October 14, 2022 05:25
@virtuald

virtuald commented Oct 14, 2022

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@virtuald

virtuald commented Oct 14, 2022

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@virtuald
bpo-41916: allow cross-compiled python to have -pthread set for CXX
fefd3d2
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.
@virtuald
virtuald force-pushed the allow-cxx-pthread-override branch from 77af732 to fefd3d2 Compare October 14, 2022 06:06
@FFY00

FFY00 commented Oct 18, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently

ericsnowcurrently commented Jan 5, 2023

Copy link
Copy Markdown
Member

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

auscompgeek reacted with hooray emoji auscompgeek reacted with heart emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead
gpshead approved these changes Jan 5, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jan 5, 2023
@gpshead
gpshead merged commit cc87487 into python:main Jan 5, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Jan 5, 2023
@virtuald virtuald mannequin mentioned this pull request Jan 5, 2023
cxx pthread check is not overrideable #86082
Closed
@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

Reviewers

@gpshead gpshead gpshead approved these changes
@FFY00 FFY00 FFY00 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@virtuald @FFY00 @kumaraditya303 @jaraco @ericsnowcurrently @gpshead @the-knights-who-say-ni @ezio-melotti @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.