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-89886: Clean up MACHDEP and _PYTHON_HOST_PLATFORM checks #30026

Closed
wants to merge 4 commits into from

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Dec 10, 2021

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 10, 2021

If When the basic CI passes, I'll send it to the bots.

@tiran
Copy link
Member

tiran commented Dec 10, 2021

There is no CI for cross building yet.

@erlend-aasland
Copy link
Contributor Author

There is no CI for cross building yet.

Let's run it on the Emscripten container then; that's a cross-build, AFAIU.

@erlend-aasland
Copy link
Contributor Author

Accidentally, I've got some ARM toolchains installed. I'll throw it at them and see how it fares.

@arhadthedev
Copy link
Member

Just out of curiosity: what distributions still don't have Autoconf so we need pregenerated configure under version control? Won't it be better to let a user or a package manager to generate it on their system instead?

@tiran
Copy link
Member

tiran commented Dec 10, 2021

Package managers are not our concern. We keep configure and other scripts in git to lower the barrier for contributors.

Our configure.ac more than just autotools. It also requires automake, a recent version of autoconf-archive and pkg-config's M4 macros. Not every distro has a recent enough autoconf archive package. Some distros have the pkg-config M4 files in a separate package. Until a couple of weeks ago our configure.ac did not work with autoconf 2.71.

@arhadthedev
Copy link
Member

Well, if this file cannot be removed because of specific requirements for its generation, maybe it can be marked as generated akin to outputs of Argument Clinic, Freeze, and Parser/asdl_c.py? It will allow GitHub to collapse it. By the way, I created #30031 to address this.

@erlend-aasland
Copy link
Contributor Author

BTW, this is a case where the configure diff matters at little bit more than normal: we have no CI for changes like this, so being able to see that the refactored code actually produces the same result is crucial. Luckily, this diff is pretty short 🙂

@erlend-aasland
Copy link
Contributor Author

Accidentally, I've got some ARM toolchains installed. I'll throw it at them and see how it fares.

Works for me.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 23, 2022
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

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

From the number of tests passed on my machine (420) I would say this looks ok to me.
But I cant test for all the supported architectures.
Also, I saw 61 fatal error in the config.log.
Is this ok?
BTW how do you check the results on configure? A part from running full test suite and checking the log.

@erlend-aasland erlend-aasland changed the title bpo-45723: Clean up MACHDEP and _PYTHON_HOST_PLATFORM checks bpo-89886: Clean up MACHDEP and _PYTHON_HOST_PLATFORM checks Apr 10, 2022
@erlend-aasland
Copy link
Contributor Author

I have no time to follow up this clean-up. Closing.

@erlend-aasland erlend-aasland deleted the ac-stuff branch June 27, 2022 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review skip news stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants