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 — gh-110014: Fix inconsistent struct definition in pycore_semaphore.h by colesbury · Pull Request #110030 · python/cpython

Conversation

@colesbury

@colesbury colesbury commented Sep 28, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

The pycore_semaphore.h header is included by Python/lock.c and Python/parking_lot.c. The macro _POSIX_SEMAPHORES was not consistently defined across the two files (due to a missing include of <unistd.h>) leading to different struct definitions. The RHEL8 ppc64le LTO buildbot correctly warned due to this issue.

  • Issue: GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@colesbury
pythongh-110014: Fix inconsistent struct definition in pycore_semapho…
6e0228b 
…re.h

The pycore_semaphore.h header is included by Python/lock.c and
Python/parking_lot.c. The macro `_POSIX_SEMAPHORES` was not consistently
defined across the two files (due to a missing include of `<unistd.h>`)
leading to different struct definitions. The RHEL8 ppc64le LTO buildbot
correctly warned due to this issue.
@colesbury colesbury added skip news build The build process and cross-build labels Sep 28, 2023
@colesbury
colesbury requested a review from vstinner September 28, 2023 14:54
@bedevere-app bedevere-app Bot added the awaiting review label Sep 28, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Sep 28, 2023
GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014
Closed
@colesbury

colesbury commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

!buildbot ppc64le
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Sep 28, 2023

Copy link
Copy Markdown

The regex 'ppc64le' did not match any buildbot builder.Is the requested builder in the list of stable builders?
colesbury reacted with confused emoji
All reactions
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

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

colesbury commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

!buildbot lto
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Sep 28, 2023

Copy link
Copy Markdown

The regex 'lto' did not match any buildbot builder.Is the requested builder in the list of stable builders?
colesbury reacted with confused emoji
All reactions
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@colesbury colesbury added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 28, 2023
@bedevere-bot

bedevere-bot commented Sep 28, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 6e0228b 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 28, 2023
@colesbury

colesbury commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author
  1. I verified locally that pycore_semaphore.h now consistently defines _PySemaphore (by temporarily added #warning)
  2. https://buildbot.python.org/all/#/builders/256/builds/1238 (ppc64, lto) does not have any warnings related to pycore_semaphore.h. It's not identical to the buildbot running on the main branch, but it's the closest configuration I could find for PRs.
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Sep 28, 2023

Copy link
Copy Markdown
Member

The regex 'ppc64le' did not match any buildbot builder.Is the requested builder in the list of stable builders?

Oh, maybe the search is case sensitive and you should use PPC64LE or PPC64LE RHEL8.
colesbury and sunmy2019 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.
@colesbury

colesbury commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

!buildbot PPC64LE
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Sep 28, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 6e0228b 🤖

The command will test the builders whose names match following regular expression: PPC64LE

The builders matched are:

  • PPC64LE Fedora Stable LTO + PGO PR
  • PPC64LE RHEL8 PR
  • PPC64LE RHEL7 Refleaks PR
  • PPC64LE RHEL7 LTO + PGO PR
  • PPC64LE RHEL8 LTO + PGO PR
  • PPC64LE Fedora Stable Clang Installed PR
  • PPC64LE Fedora Stable LTO PR
  • PPC64LE RHEL7 PR
  • PPC64LE Fedora Stable PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE RHEL7 LTO PR
  • PPC64LE RHEL8 Refleaks PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE RHEL8 LTO PR
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Sep 28, 2023

Copy link
Copy Markdown
Member

Oh, maybe the search is case sensitive and you should use PPC64LE or PPC64LE RHEL8.

I wrote python/buildmaster-config#409 to make the search ignores the case.

I got bitten multiple times by the !buildbot command which said nothing. So I added the "The regex 'lto' did not match any buildbot builder.Is the requested builder in the list of stable builders?" message (I just added the missing space ;-)).
All reactions

Sorry, something went wrong.

Uh oh!

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

colesbury commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

Thanks!
vstinner and sunmy2019 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.
@colesbury

colesbury commented Sep 29, 2023

Copy link
Copy Markdown
Contributor Author

@vstinner, would you please review this when you get a chance?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner requested changes Sep 30, 2023
View reviewed changes

@vstinner vstinner 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.

Oh wait. The problem is way wider than pycore_semaphore.h! _POSIX_THREADS and _POSIX_SEMAPHORES macros are checked in Python/pthread_thread.h, pycore_condvar.h. and pycore_pythread.h without explicitly including <unistd.h>.

I wrote a wider change to attempt to fix all cases: PR gh-110139.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Sep 30, 2023
@vstinner

vstinner commented Sep 30, 2023

Copy link
Copy Markdown
Member

I wrote a more complete fix: PR #110139. Thanks for working on fixing these warnings!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner closed this Sep 30, 2023
@colesbury
colesbury deleted the issue_110014 branch October 4, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner requested changes

Uh oh!

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

Assignees

No one assigned

Labels

awaiting core review build The build process and cross-build skip news

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.

3 participants

@colesbury @bedevere-bot @vstinner
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.