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-38021: Modify AIX platform_tag so it provides PEP425 needs #17303

Merged
merged 14 commits into from Dec 15, 2019
Merged

bpo-38021: Modify AIX platform_tag so it provides PEP425 needs #17303

merged 14 commits into from Dec 15, 2019
+155 −5

Conversation

@aixtools
Copy link
Contributor

aixtools commented Nov 21, 2019 •
edited

PEP425 defines the platform_tag as whatever sysconfig.get_platform() (and distutils.util.get_platform() returns). HOWEVER, for a platform_tag to be useable it needs to satisfy a number of conditions.

The main condition is to provide sufficient detail so that binary distributions (e.g., eggs and wheels)
can be applied, accepted or rejected by distribution tools (e.g, pip and wheel).

This PR specifies the AIX platform tag be specified (effectively) as:
"AIX-{:04d}-{:04d}-{:2d}".format(vrtl,builddate,bitsize)

e.g., AIX 5.3 TL7 SP0 would be identified as: "AIX-5307-0747-32" and "AIX-5307-0747-64" for 32bit and 64bit builds, respectively.

Most of the information needed to provide a "pep425 build tag" is already available:
sysconfig.get_config_var("GNU_BUILD_TYPE") and sys.maxsize.

Comparable to how macos can determine it build-system characteristics one additional variable is needed: sysconfig.get_config_var("AIX_BUILDDATE").

As AIX guarantees application binary compatibility from old to new the run-time value of the get_platform() can be compared with the aix_buildtag() to determine whether a bdist is acceptable for installation, or not.

https://bugs.python.org/issue38021
@aixtools
Provide a platform tag for AIX sufficient PEP425 binary distribution …
Loading status checks…
e447767 
…identification
@ned-deily ned-deily requested review from brettcannon and ncoghlan Nov 21, 2019
@ned-deily

This comment has been minimized.

Sign in to view
Copy link
Member

ned-deily commented Nov 21, 2019

Adding @brettcannon and @ncoghlan to review in particular regarding input from PyPA (see bpo issue comments)
@brettcannon brettcannon removed their request for review Nov 21, 2019
@brettcannon

This comment has been minimized.

Sign in to view
Copy link
Member

brettcannon commented Nov 21, 2019

I'm not the right person for distutils stuff.
@ned-deily

This comment has been minimized.

Sign in to view
Copy link
Member

ned-deily commented Nov 21, 2019

Additional comment here: https://bugs.python.org/issue38021#msg357236
@aixtools
Add docmentation to ``distutils.apiref.get_platform()``
Loading status checks…
c7b611c 
Change field seperator in tag from to ``'-'`` to be similiar with other tags
@webknjaz
webknjaz reviewed Nov 22, 2019
View changes
Lib/_aix_support.py Outdated Show resolved Hide resolved
Lib/_aix_support.py Outdated Show resolved Hide resolved
Lib/_aix_support.py Outdated Show resolved Hide resolved
Lib/_aix_support.py Outdated Show resolved Hide resolved
Lib/sysconfig.py Outdated Show resolved Hide resolved
aixtools added 2 commits Nov 22, 2019
@aixtools
Be ready for AIX 10.X, should it ever come!
Loading status checks…
a681a9d
@aixtools
one base source for CPython, pypa/pip and pypa/packaging
Loading status checks…
d37a63b
@aixtools aixtools changed the title bpo-38021: Provide a platform tag for AIX that satisfies PEP425 demands bpo-38021: Modify AIX platform_tag so it provides PEP425 needs Nov 28, 2019
aixtools added 3 commits Nov 30, 2019
@aixtools
move all bootstrap issues into support module
Loading status checks…
897a285
@aixtools
correct typo and reword description
911b4d9
@aixtools
reword description for AIX_BUILDDATE
Loading status checks…
53ffd14
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Dec 1, 2019

Hoping for a final review. :)

Entering the year-end dash at work, so I may be slow to reply.
@aixtools aixtools mentioned this pull request Dec 1, 2019
Open
@ncoghlan

This comment has been minimized.

Sign in to view
Copy link
Contributor

ncoghlan commented Dec 8, 2019

@aixtools Given that, I'm going to go ahead and make the documentation changes necessary to mark this as a 3.9+ only feature.
@ncoghlan
ncoghlan requested changes Dec 8, 2019
View changes
Copy link
Contributor

ncoghlan left a comment

I'm in the process of making some direct edits for documentation wording and markup changes, but there's a more substantial change needed before we accept the PR: to minimise compatibility risks, the platform prefix should remain in lowercase, and any code that cares about the rest of the formatting will need to switch based on the number of hyphens found.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 8, 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.
ncoghlan and others added 3 commits Dec 8, 2019
@ncoghlan
Adjust doc and NEWS wording
Loading status checks…
35cf9a1
@ncoghlan
Wording fix in versionchanged note
Loading status checks…
c51187f
@aixtools
changes per peer-review
Loading status checks…
6b94003
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Dec 8, 2019

I have made the requested changes; please review again - both in code and docs. AIX tag is lowercase.

Shall adjust my PR in pypa to switch based on the number of hyphens found.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 8, 2019

Thanks for making the requested changes!

@ncoghlan: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from ncoghlan Dec 8, 2019
aixtools added a commit to aixtools/packaging that referenced this pull request Dec 8, 2019
@aixtools
Changes made in accordance of peer review of CPyhton PR #17303
b46efe0 
See python/cpython#17303 (review)
ncoghlan added 2 commits Dec 15, 2019
@ncoghlan
Cosmetic fixes to AIX support code
cc9011a
@ncoghlan
Restore Debian runstatedir to checked in configure script
Loading status checks…
d8a3a2d
@ncoghlan
ncoghlan approved these changes Dec 15, 2019
View changes
@ncoghlan

This comment has been minimized.

Sign in to view
Copy link
Contributor

ncoghlan commented Dec 15, 2019

@aixtools When I sent my last review, it was missing some cosmetic comments related to the error handling logic in the _aix_support module. Since I missed sending those last time, I went ahead and made them directly, rather than taking you through yet another review-edit-review cycle.
ncoghlan added 2 commits Dec 15, 2019
@ncoghlan
Fix typo
d68b04b
@ncoghlan
Further tweak to docs wording
Loading status checks…
fb111ca
@ncoghlan ncoghlan merged commit 39afa2d into python:master Dec 15, 2019
4 checks passed
4 checks passed
Azure Pipelines PR #20191215.10 succeeded
Details
bedevere/issue-number Issue number 38021 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@aixtools

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

aixtools commented Dec 15, 2019

@aixtools When I sent my last review, it was missing some cosmetic comments related to the error handling logic in the _aix_support module. Since I missed sending those last time, I went ahead and made them directly, rather than taking you through yet another review-edit-review cycle.

Thanks!
sthagen added a commit to sthagen/cpython that referenced this pull request Dec 15, 2019
@sthagen
Merge pull request #99 from python/master
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…
064e8e3 
bpo-38021: Modify AIX platform_tag so it covers PEP 425 needs (pythonGH-17303)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz

@ncoghlan ncoghlan

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
7 participants
@aixtools @ned-deily @brettcannon @ncoghlan @bedevere-bot @webknjaz @the-knights-who-say-ni
You can’t perform that action at this time.