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-89886: Clean up MACHDEP and _PYTHON_HOST_PLATFORM checks by erlend-aasland · Pull Request #30026 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Dec 10, 2021 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue45723

Sorry, something went wrong.

Uh oh!

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

All reactions
Erlend E. Aasland added 3 commits December 10, 2021 13:07
Erlend E. Aasland
bpo-45723: Clean up MACHDEP check
6e60f4c
Erlend E. Aasland
bpo-45723: Clean up _PYTHON_HOST_PLATFORM check
ac156c8
Erlend E. Aasland
Fix error message
7419e1c
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Dec 10, 2021
@erlend-aasland
erlend-aasland requested a review from tiran December 10, 2021 12:16
@bedevere-bot bedevere-bot added the awaiting review label Dec 10, 2021
@erlend-aasland erlend-aasland added the skip news label Dec 10, 2021
@erlend-aasland

erlend-aasland commented Dec 10, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

Erlend E. Aasland
Normalize capitalization in error message
e675a06
@tiran

tiran commented Dec 10, 2021

Copy link
Copy Markdown
Member

There is no CI for cross building yet.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Dec 10, 2021

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev

arhadthedev commented Dec 10, 2021

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@tiran

tiran commented Dec 10, 2021

Copy link
Copy Markdown
Member

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.

erlend-aasland and arhadthedev reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev

arhadthedev commented Dec 10, 2021

Copy link
Copy Markdown
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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Dec 10, 2021

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Dec 22, 2021

Copy link
Copy Markdown
Contributor Author

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

Works for me.

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions

github-actions Bot commented Jan 23, 2022

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jan 23, 2022
MaxwellDupre
MaxwellDupre approved these changes Feb 27, 2022
View reviewed changes

@MaxwellDupre MaxwellDupre left a comment

Copy link
Copy Markdown
Contributor

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.

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.

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 Feb 27, 2022
@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
@tiran tiran mentioned this pull request Apr 22, 2022
Improve and simplify configure.ac checks #89886
Closed
@erlend-aasland

erlend-aasland commented Jun 27, 2022

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland erlend-aasland closed this Jun 27, 2022
@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

Reviewers

@tiran tiran Awaiting requested review from tiran
1 more reviewer
@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

awaiting core review skip news stale Stale PR or inactive for long period of time.

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.

6 participants

@erlend-aasland @tiran @arhadthedev @MaxwellDupre @the-knights-who-say-ni @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.