Skip to content

/ cpython

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-36210: correct logic in setup.py for optional extensions with regard to AIX #12202

Merged
merged 12 commits into from Jun 21, 2019
Merged

bpo-36210: correct logic in setup.py for optional extensions with regard to AIX #12202

merged 12 commits into from Jun 21, 2019
+26 −8

Conversation

@aixtools
Copy link
Contributor

aixtools commented Mar 6, 2019 •
edited

There are three optional modules that either will never exist (spwd and osaudiodev) or would build correctly if the OS provided library was preferred rather than an optionally installed, non-supported 3rd-party library (ncurses).

  • This PR sets AIX to build _curses against the IBM supported /usr/lib/libcurses.a and skip the optional _cursus_panel.

    • Python _curses module builds with no issues from AIX 5.3 through AIX 7.2 when using the officially supported libcurses.a
    • When a third-party packaging of ncurses is installed neither _curses nor _cursus_panel build successfully.

  • setup.py no longer says _spwd and _osaudiodev are missing (something) when building for AIX

https://bugs.python.org/issue36210
@aixtools
Switch to officially support curses from 3rd-party ASIS supported ncu…
Loading status checks…
ccf40a6 
…rses
@ZackerySpytz
ZackerySpytz reviewed Mar 31, 2019
View changes
setup.py Outdated Show resolved Hide resolved
aixtools added 5 commits Apr 1, 2019
@aixtools
line was too long [PEP 8], now two lines
Loading status checks…
178a739
@aixtools
clarify change and remove repeated boolean logic
Loading status checks…
f29cf2c
@aixtools
boolean logic over two lines needs a backslash on first line
4d74e70
@aixtools
stop saying optional modules osaudiodev and spwd are missing on AIX
401a266
@aixtools
update News blurb
Loading status checks…
f56b764
@aixtools aixtools changed the title bpo-36210: Switch to officially support curses from ASIS supported ncurses bpo-36210: correct AIX logic in setup.py for optional extensions Apr 2, 2019
@aixtools aixtools changed the title bpo-36210: correct AIX logic in setup.py for optional extensions bpo-36210: correct logic in setup.py for optional extensions with regard to AIX Apr 2, 2019
aixtools added 3 commits Jun 20, 2019
@aixtools
Merge branch 'master' into bpo-36210-ncurses
b922583
@aixtools
Modify inline comments and clarify logic
Loading status checks…
c7ca6fd
@aixtools
Add missing backslash
Loading status checks…
7c6b2e7
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Jun 20, 2019

Question: what is the current status of spwd module in Python? Is this something I should look at seeing if I can emulate in AIX (i.e., not using getspent() etc, functions, but the AIX ones for accessing shadow information).

I thought I heard that spwd will be removed - sometime.

If spwd is going to stay forever, then the missing announcement may be useful, but asis, it is just some noise.
@aixtools
Fix whitespace
Loading status checks…
d240053
@aixtools aixtools closed this Jun 20, 2019
@aixtools aixtools reopened this Jun 20, 2019
@ncoghlan

This comment has been minimized.

Sign in to view
Copy link
Contributor

ncoghlan commented Jun 21, 2019

spwd is one of the libraries on the removal list in PEP 594: https://www.python.org/dev/peps/pep-0594/#spwd
@ncoghlan
Update 2019-03-06-18-55-10.bpo-36210.fup9H2.rst
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a0117e0
@ncoghlan
ncoghlan requested changes Jun 21, 2019
View changes
Copy link
Contributor

ncoghlan left a comment

This mostly seems fine to me, but I'd prefer to update the curses_panel logic to make it more obviously free of potential side effects on other platforms.
setup.py Outdated Show resolved Hide resolved
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jun 21, 2019

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.
@aixtools
use variable skip_curses_panel with AIX
Loading status checks…
2572144
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Jun 21, 2019

I have made the requested changes; please review again
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jun 21, 2019

Thanks for making the requested changes!

@ncoghlan: please review the changes made to this pull request.
@ncoghlan
ncoghlan approved these changes Jun 21, 2019
View changes
@ncoghlan ncoghlan merged commit 08970cb into python:master Jun 21, 2019
5 checks passed
5 checks passed
Azure Pipelines PR #20190621.14 succeeded
Details
bedevere/issue-number Issue number 36210 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jun 21, 2019

Thanks @aixtools for the PR, and @ncoghlan for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
lisroach added a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@aixtools @lisroach
bpo-36210: update optional extension handling for AIX (pythonGH-12202)
24a479b 
* Switch to officially supported curses from 3rd-party ASIS supported ncurses
* stop saying optional modules osaudiodev and spwd are missing on AIX

Patch by M.Felt
aixtools added a commit to aixtools/cpython that referenced this pull request Sep 25, 2019
@aixtools
bpo-36210: correct logic in setup.py for optional extensions for AIX (p…
c53cfaf 
…ythonGH-12202)

patch by M Felt
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Sep 25, 2019

Added #16376 as backport.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Sep 25, 2019

GH-16376 is a backport of this pull request to the 3.8 branch.
ned-deily added a commit that referenced this pull request Sep 25, 2019
@aixtools @ned-deily
bpo-36210: correct logic in setup.py for optional extensions for AIX (G…
Loading status checks…
e4be8c9 
…H-12202) (GH-16376)
DinoV added a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@aixtools @DinoV
bpo-36210: update optional extension handling for AIX (pythonGH-12202)
300e76a 
* Switch to officially supported curses from 3rd-party ASIS supported ncurses
* stop saying optional modules osaudiodev and spwd are missing on AIX

Patch by M.Felt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZackerySpytz ZackerySpytz

@ncoghlan ncoghlan

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
6 participants
@aixtools @ncoghlan @bedevere-bot @miss-islington @ZackerySpytz @the-knights-who-say-ni
You can’t perform that action at this time.