bpo-46053: Fix OSS audio support on NetBSD

[0-wiz-0]

https://bugs.python.org/issue46053

[tiran]

We keep feature checks and module definitions separate. Between LIBNSL and LIBSQLITE3 looks like a nice spot. We also prefer m4 macros for new code:

AS_CASE([$ac_sys_system],
  [NetBSD*], [OSSAUDIODEV_LIBS="-lossaudio"],
  [OSSAUDIODEV_LIBS=""]
)

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[0-wiz-0]

Thanks for the suggestions!

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

[github-actions]

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

[0-wiz-0]

@serhiy-storchaka You were so kind to review my other NetBSD pull requests, can you please check this one as well?

[serhiy-storchaka]

I am not experienced with autotools. Should not AC_CHECK_LIB be used here?

[0-wiz-0]

AFAIK only NetBSD provides libossaudio, and all versions of it (but definitely all supported versions) provide the library. That's why I thought it'd be easier just to add it this way.

[serhiy-storchaka]

It should be in the "Library" section, not in "Core and Builtins". And please add a sentence-ending period.

Otherwise LGTM.

[0-wiz-0]

Thanks for the review, @serhiy-storchaka
I've made the requested changes.

[hugovk]

Note the ossaudiodev module is deprecated in 3.11 and set for removal in 3.13.

See PEP 594 – Removing dead batteries from the standard library and #91217.

[AlexWaygood]

Note the ossaudiodev module is deprecated in 3.11 and set for removal in 3.13.

See PEP 594 – Removing dead batteries from the standard library and #91217.

There is also now a merge conflict (I don't know how large the merge conflict is).

[hugovk]

Here's the conflict:

diff --cc configure.ac
index 7d194f263e,6a460cd94c..0000000000
--- a/configure.ac
+++ b/configure.ac
@@@ -6409,8 -6789,8 +6795,13 @@@ PY_STDLIB_MOD([_socket]
  dnl platform specific extensions
  PY_STDLIB_MOD([grp], [], [test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes])
  PY_STDLIB_MOD([ossaudiodev],
++<<<<<<< HEAD
 +  [], [test "$ac_cv_header_linux_soundcard_h" = yes -o "$ac_cv_header_sys_soundcard_h" = yes],
 +  [], [$OSSAUDIODEV_LIBS])
++=======
+   [], [test "$ac_cv_header_linux_soundcard_h" = yes -o "$ac_cv_header_sys_soundcard_h" = yes])
+ PY_STDLIB_MOD([pwd], [], [test "$ac_cv_func_getpwuid" = yes -o "$ac_cv_func_getpwuid_r" = yes])
++>>>>>>> upstream/main
  PY_STDLIB_MOD([resource], [], [test "$ac_cv_header_sys_resource_h" = yes])
  PY_STDLIB_MOD([_scproxy],
    [test "$ac_sys_system" = "Darwin"], [],

[0-wiz-0]

Some more years will pass before this code is really removed from python, so I'd like to see this merged.

I tried fixing the merge conflict on the ossaudio branch but my git-foo left me, the merged commits are now on

https://github.com/0-wiz-0/cpython/tree/ossaudio-v2

instead (but it's straightforward - on main, a line was added that is in the context of the change I made, the context just needs to be adapted).

[hugovk]

If https://github.com/0-wiz-0/cpython/tree/ossaudio-v2 has what you want to merge, two options:

• Create a new PR from that branch, close this

• Reset this branch to that one, and force push to replace this:

# Start from this branch
git checkout ossaudio

# Replace the contents of this branch with ossaudio-v2
git reset --hard ossaudio-v2

# Now because we've rewritten the history of this branch, 
# we need to force push it
git push --force

[0-wiz-0]

Thanks for the instructions, @hugovk ! I've just force-pushed to the ossaudio branch.

[AlexWaygood]

@serhiy-storchaka, you previously approved this PR — reckon it can now be merged? :)

[serhiy-storchaka]

I think that it is worth to to continue to fix bugs in deprecated modules. Especially if the solution is already ready.

Requests for new features will be rejected of course.

[0-wiz-0]

Thank you for merging this, @serhiy-storchaka !

[miss-islington]

Thanks @0-wiz-0 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

[miss-islington]

Thanks @0-wiz-0 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

[miss-islington]

Sorry, @0-wiz-0 and @serhiy-storchaka, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2e7e3c4c109928870c1e33d8af36b78e92895594 3.9

[miss-islington]

Sorry @0-wiz-0 and @serhiy-storchaka, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 2e7e3c4c109928870c1e33d8af36b78e92895594 3.10

[serhiy-storchaka]

@0-wiz-0 do you think it is a good idea of backporting this change?

[0-wiz-0]

@serhiy-storchaka if it's easy, I'd appreciate it (I think it should be easy) but if you don't want to spend the time on this, I understand and it's fine with me.

[serhiy-storchaka]

I mean, do you want to spent your time on this? 😉 I'll approve it immediately.

It is okay if you do not do it. I can make the backport myself, although I am not enough motivated to test it on NetBSD.

[bedevere-bot]

GH-95477 is a backport of this pull request to the 3.10 branch.

[0-wiz-0]

Hi @serhiy-storchaka ! Sorry it tooks so long, but I've finally tried cherry_picker. The problem was that 3.10 and earlier still have the ossaudio module support in setup.py instead of configure.ac.
The pull request for 3.10 is here:
#95477
works on NetBSD

If this is fine, can we use that patch to merge to 3.9? I think that should apply more easily to 3.9.
Thanks.

[AlexWaygood]

If this is fine, can we use that patch to merge to 3.9?

3.9 is now in security-only mode; only security-related patches are backported to 3.9 these days, I'm afraid.

[0-wiz-0]

@AlexWaygood Oh, that's fine then. Thanks for the information.