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.

Sign up for GitHub

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

Jump to bottom

gh-89452: Prefer gdbm-compat over ndbm #92208

Merged
merged 1 commit into from
May 3, 2022
Merged

gh-89452: Prefer gdbm-compat over ndbm #92208

merged 1 commit into from
May 3, 2022

Conversation

ambv
Copy link
Contributor

@ambv ambv commented May 3, 2022 •
edited
Loading

This makes macOS gdbm provided by Homebrew not segfault through correct
selection of the linked library (-lgdbm_compat) AND the correct ndbm-style
header (gdbm-ndbm.h instead of the invalid ndbm.h).

Addresses #89452.

@ambv
pythongh-89452: Prefer gdbm-compat over ndbm
f360ff5 
This makes macOS gdbm provided by Homebrew not segfault through correct
selection of the linked library (-lgdbm_compat) *AND* the correct ndbm-style
header (gdbm-ndbm.h instead of the invalid ndbm.h).
@ambv ambv added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 3, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ambv for commit f360ff5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 3, 2022
ambv
ambv commented May 3, 2022
View reviewed changes
Modules/_dbmmodule.c
@@ -24,6 +21,9 @@
#error "No gdbm/ndbm.h or gdbm-ndbm.h available"
#endif
static const char which_dbm[] = "GNU gdbm";
#elif defined(USE_NDBM)
#include <ndbm.h>
static const char which_dbm[] = "GNU gdbm";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the now stale comment about the OS/2 "EMX port" that was added over 20 years ago.

@ambv ambv requested review from tiran and ned-deily May 3, 2022 01:42
@ambv ambv linked an issue May 3, 2022 that may be closed by this pull request
test_dbm and test_dbm_ndbm segfaults in M1 Mac with Homebrew gdbm 1.23 #89452
Closed
@ambv ambv removed a link to an issue May 3, 2022
test_dbm and test_dbm_ndbm segfaults in M1 Mac with Homebrew gdbm 1.23 #89452
Closed
@ambv
Copy link
Contributor Author

ambv commented May 3, 2022

(I unlinked the PR from the issue because I don't want the PR to close the issue. We need backports first.)

@ambv
Copy link
Contributor Author

ambv commented May 3, 2022

The two failing RHEL7 buildbots are SIGABRT of an embed process due to an assertion failure like:

_testembed: Python/specialize.c:266: _PyCode_Quicken: Assertion `instructions[i + 1] == 0' failed.

This doesn't look related to my changes here.

@ambv ambv added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 3, 2022
@ambv ambv merged commit 6c25bf0 into python:main May 3, 2022
@miss-islington
Copy link
Contributor

Thanks @ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @ambv, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 6c25bf07e86b6d8d5e50488835f70539f382d7e0 3.10

@miss-islington
Copy link
Contributor

Sorry @ambv, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 6c25bf07e86b6d8d5e50488835f70539f382d7e0 3.9

@ambv ambv deleted the gh89452 branch May 3, 2022 16:28
@ambv ambv restored the gh89452 branch May 3, 2022 16:41
@ambv ambv deleted the gh89452 branch May 3, 2022 16:41
@ambv
Copy link
Contributor Author

ambv commented May 3, 2022

OK, looks like the build-related changes made by Christian in #29467 and #29720 were only made to 3.11 so I'm aborting backports here.

@ambv ambv removed needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 3, 2022
@encukou encukou mentioned this pull request Oct 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ned-deily ned-deily ned-deily approved these changes

@tiran tiran Awaiting requested review from tiran

Assignees

@ambv ambv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ambv @bedevere-bot @miss-islington @ned-deily