-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
Modify AIX platform_tag so it provides PEP425 needs #82202
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
Comments
PEP-425 stats the platform tag is what distutils.util.get_platform() (and sysconfig.get_platform()) returns. By that definition - anything is okay, as long as something is returned. However, in practice, it is insufficient. Simplest case - there is no indication of the fitness of the running Python. This, by itself, may be a reason that VARs and others have never attempted to develop binary (compatible) eggs or wheels. Looking further at missed potential - AIX has an algorithm that can be used to permit a forward (binary) compatible ABI for packaging. The current state of packaging for AIX is to create an RPM or INSTALLP formatted package - and install that. Even pure-python modules must be packaged in this way - to satisfy the "binary" dependencies. For a binary compatible algorithm to be possible changes will be needed in pypa tools. However, rather than "patch" three sets of tools to correct the inadequacy of the current PEP-425 tag for AIX I propose to tag AIX as: where v=version, r=release, tl=technology_level, bd=builddate, and sz=fitness (32|64). Further, rather than place this code "inline" in distutils or sysconfig create a new support module for AIX - similar to the _osx_support module (I.e. _aix_support). |
I don't have an opinion about the bulk of the proposed change. But, since _osx_support was mentioned as a model for an _aix_support, I wanted to comment on the history of _osx_support. _osx_support was created as a separate module back when the distutils2 (aka packaging) project was active and it appeared that, at least for some number of releases, both distutils and distutils2 would have to co-exist in the cpython source repo. So, rather than duplicating the macOS code - most of which had been added on to distutils piecemeal over the years - in both places, we ripped out the existing code from distutils and put it into the new _osx_support module for use by both. In the end, though, a different direction for distutils was chosen and the distutils2/packaging project code was removed from cpython prior to a formal release. If we were doing it today, there probably would be no reason to have a separate _osx_support module as nearly all of the code is only called from one place. Thus, I would not use its existence as a justification for having a separate _aix_support module. |
Thank you Ned. Not a justification perhaps, but a way to specify that it is support. Three+ years ago when I first worked on something for Lib/ctypes to get find_library() et al working for AIX I was also asked to add it as _aix.py similar to the macos support (with, iirc is more than just a single file) being separate from util.py. To me, it just seemed that is the way it is done to keep maintenance of the core (external) code cleaner. After I post the PR I welcome your perspective on the sense or nonsense of having a separate _xxx_support.py file (where _xxx could be any platform, not merely osx or aix. |
When testing the PR with --with-pydebug I started getting the following error: root@x066:[/data/prj/python/git/pr-test]./python '-X' 'tracemalloc' -m test test_venv
Run tests sequentially
0:00:00 [1/1] test_venv
test test_venv failed -- Traceback (most recent call last):
File "/data/prj/python/git/pr-test/Lib/test/test_venv.py", line 501, in test_with_pip
self.do_test_with_pip(False)
File "/data/prj/python/git/pr-test/Lib/test/test_venv.py", line 459, in do_test_with_pip
self.assertEqual(err, "")
AssertionError: '/data/prj/python/git/pr-test/Lib/_aix_sup[277 chars]t,\n' != ''
- /data/prj/python/git/pr-test/Lib/_aix_support.py:47: ResourceWarning: unclosed file <_io.TextIOWrapper name=4 encoding='ISO8859-1'>
- return aix_pep425()
- Object allocated at (most recent call last):
- File "/data/prj/python/git/pr-test/Lib/subprocess.py", lineno 837
- self.stdout = io.TextIOWrapper(self.stdout, test_venv failed in 34 sec 401 ms == Tests result: FAILURE == 1 test failed: Total duration: 34 sec 485 ms +++ capturing some debug output: out:pip 19.2.1 from /tmp/tmppcl06489/lib/python3.9/site-packages/pip (python 3.9) I thought - that the call p.wait() was waiting for the process to complete (and close files). Note: changing p.wait() to p.wait is not the solution: the message changes to:
root@x066:[/data/prj/python/git/pr-test]cat /tmp/test_venv.debug out:pip 19.2.1 from /tmp/tmp8wpnz193/lib/python3.9/site-packages/pip (python 3.9) I was able to resolve the issue by changing Popen() to run(): oot@x066:[/data/prj/python/git/pr-test]diff -u /data/prj/python/git/pr-test/Lib/_aix_support.py /tmp/_aix_support.py import sys
- from subprocess import Popen, PIPE, DEVNULL
+ from subprocess import run
from sysconfig import get_config_var as get_var
_is_32bit = sys.maxsize == 2147483647
@@ -32,10 +32,9 @@
The pep425 platform tag for AIX becomes:
AIX.VRTL.YYWW.SZ, e.g., AIX.6107.1415.32
"""
- p = Popen(["/usr/bin/lslpp", "-Lqc", "bos.mp64"],
- universal_newlines=True, stdout=PIPE, stderr=DEVNULL)
- _lslppLqc = p.stdout.read().strip().split(":")
- p.wait
+ p = run(["/usr/bin/lslpp", "-Lqc", "bos.mp64"],
+ capture_output=True, text=True)
+ _lslppLqc = p.stdout.strip().split(":")
Don't know if this is a bug in Popen(), or just an error in my use of Popen(). Happy that using run() solves it! |
PEP-425 tags are now in the Python Packaging Authority realm. People from there should also be reviewing this issue and the new PR. CC-ing Brett and Nick for guidance. Also, assuming the feature is approved, I don't think it would be appropriate to backport to 3.7 (considering where 3.7 is in its life cycle). It may be appropriate to make an exception for 3.8 since it is still early in its life. |
I'm not in a good position to review distutils stuff. |
Brett, sorry I wasn't more explicit about my concerns here. I added you and Nick to this issue and to its PR not primarily to do do code review but because I think it brings up issues that I believe haven't been dealt with before in our evolving environment and may need guidance from the Steering Council and the PyPA, both of which you two have been involved in. The PR is about defining new platform tags, something which has potential impact across both CPython development (it can affect how Python itself is built) and across the packaging realm (as any packaging tool like, presumably, pip and wheel and others are potentially affected). This also is relevant to recent discussions about the roles of PyPA and CPython development. Because it has the portential to impact and require changes across the board, we need to make sure that proposed changes like this get adequate review and consensus across the board. I'm not exactly sure how that should work so I think it's something that is worth clarifying. AIX is already a supported platform, at least on best effort basis thanks in large part to the efforts of non-core developers like Michael here (and thank you, Michael). So, in this case, the impact is probably most on the PyPA side so I think there needs to be their explicit involvement. But, for example, there is a risk of a similar PR adding new platform support without adequate review which could have a major long-term impact on CPython maintenance and support (and such things have happened in the past). Perhaps we need to formulate an explicit policy about approval of changes to cpython that affect platform support and thus packaging. |
a) - thanks Ned, for the kind words. b) - the proposed (change to the tag) is "AIX.VRTL.YYWW.SZ". The builddate is crucial as system updates are possible, in oslevel -s terms, from 6100-09-11-1810 to 7100-05-02-1810 but not from 6100-09-12-1846 to 7100-05-02-1810. Using the proposed PEP-425 tag the VRTL-YYWW numbers here would be: 6109-1810, 6109-1846 and 7105-1810. The build-date of the executable is essential, as it specifies the lowest date of any AIX release that it is supported on. This is something CPython must provide, as it does for other platforms, in the form of sysconfig.get_config_var("AIX_BUILDDATE"). The VRTL values of the executable are already available via sysconfig.get_config_var("BUILD_GNU_TYPE") while SZ is typically determined from sys.maxsize. Happy to answer additional questions - and write additional .rst documentation as desired. e.g., an |
PyPA member here - if this PR is defining new compatibility tags, I would have expected it to need discussion as a revision to PEP-425, much like the manylinux efforts did. (It may actually be a closer parallel with MacOS, which didn't have a tagging PEP - I'm not sure how that would relate here, but as MacOS is a much more mainstream platform I'd be inclined to err on the side of caution and look for AIX to be explicitly covered in the tag specs). I thought that was something that had been discussed on the Pip tracker, but maybe the implications weren't clear (I don't really understand the AIX scheme, so I'm relying on someone else, probably Michael, to propose something that could be added to PEP-425 and to lead/guide the discussion). |
@paul.moore - thanks for the comment. I am trying to work from both https://packaging.python.org/specifications/platform-compatibility-tags/ which describes in a few words the goals of PEP-425. As to the PEP-425 itself, it does not specify what a tag looks like. Within the EPE the only description is: The platform tag is simply distutils.util.get_platform() with all hyphens - and periods . replaced with underscore _. Like the platform-compatibility-tags reference says - the rational (aka requirement) "distributions should have a file naming convention that includes enough information to decide whether or not a particular archive is compatible with a particular implementation." The tag I am proposing 'includes enough information'. Further, as to the comparison of Linux and macos approaches to tags and deciding "whether or not a particular archive is compatible" AIX system architecture does not have to deal with the diversity that Linux platforms do - so it would be closer to macos approach. Actually, deciding if a bdist is compatible or not will not be the hard part. However, I prefer to discuss that after a starting point - the platform tag itself - is achieved. The last bit I am looking into is how, e.g., |
What complicates the issue for AIX (and reminds me very strongly of the MacOS and manylinux situations, both of which were defined after the original tag PEP, and so contain additional insights) is the business of certain tags being compatible across multiple AIX versions. That's something that needs to be fairly clearly specified, so that implementors and maintainers understand the design. And even more so for a niche platform like AIX, as we can't rely on a platform expert being available. (Consider for example a pip issue "this wheel says it's compatible with AIX x.y.z, my machine is AIX x.y.w which is documented as compatible, why isn't it working?") It's possible that all of this may not have any relevance to the specific change to core Python, but it's hard to be sure of that when there's nothing other than your explanation to go on. A tagging spec would act as a clear reference to work from (even if it's basically just you writing up your knowledge, doing so would at least allow people to say "hey - I don't follow that bit, can you clarify"). To put it another way, you need somebody to sign off on this change as correct. You'll have an easier time of getting that if there's a written up spec that Python developers can refer to, without having to go back to (and understand) all of the AIX source material that it's based on. |
For folks that aren't aware, Michael and I have been discussing the AIX package tagging problem via email since his initial distutils-sig posts about it. It's genuinely murky as fixing the AIX problem doesn't *technically* require any PEP-425 changes, as the main problem with the status quo is in the way the platform string is calculated, rather than in the way that then gets incorporated into a PEP-425 compatibility tag. I think the subtlety I may not have cconveyed clearly though is that installers only do exact string matches on platform tags, working through a set of increasingly less specific candidates. So if the goal is the same model that we use for Windows and Mac OS X, where the Python interpreter build defines the exact platform string that packages need to use, then all that's needed is the new platform string logic. That then places constraints on how AIX extension modules are built, but doesn't require a new PEP for installer logic changes. I think this is a good level of support to target. There's then a further option that *would* require a new PEP: teaching installers to be more permissive in the compatibility tags they accept for AIX, based on the AIX ABI compatibility guidelines. I *don't* think that's a good idea - better to deal with that on the publication side, the way it is done for Windows and Mac OS X extensions. |
Thanks for the clarification, Nick. Yes, I agree that the basic "this is the tag that matches this precise CPython installation" is not the difficult part of the problem here, and if that is all that this issue is targeting, there should be no major problem. My understanding was that Michael wanted to address the other part of the issue, that installers would somehow need to encapsulate the question of what binaries were compatible with what installations, and that, as you say, is a *much* murkier question. If you're comfortable that the two aspects can be cleanly separated (and ideally that the second can be avoided altogether) then I'm much happier. |
On 22/11/2019 10:42, Paul Moore wrote:
As I understand the 'manylinux*' compatibility tags - which are used as Before AIX 6.1 binary compatibility in AIX was not guaranteed upfront. From https://www.recarta.co.uk/our-partners/software-partners/ibm-aix/ This page, from IBM, speaks of binary compatibility even going back to So, again - the PR is to provide an adequate platform tag. As is, an I am willing to work on an informational PEP - as long as waiting for Please note: the PR does not address any aspect of point b). Without Sincerely, Michael
|
Ah, I see, sorry. In that case, this should be fine, it's purely a CPython question. There's obviously a follow-on discussion about how that platform tag is *incorporated* into the compatibility tags, but as you say, that's a separate point. |
On 26/11/2019 20:10, Paul Moore wrote:
|
Updated this PR, and PRs in pypa/pip and pypa/packaging to all be "in sync". |
Removing 3.9 from the target versions, as similar to other platform tag improvements, emulation on older release versions will be the domain of cross-version libraries, rather than changing the standard library in a maintenance. |
There's a compatibility problem with changing the AIX distutils platform prefix from aix to AIX: any existing code that does "distutils.get_platform().startswith('aix')" will break. (There isn't any code in the standard library that does that, it all checks sys.platform() instead, but it's a reasonable assumption that there's going to be code in the wild that does a prefix check on the distutils API output). So I think we'll want to make the distinction between the two formats based on the number of hyphens they contain, rather than changing the prefix. I *haven't* made that change directly to the PR myself, as I want to give you a chance to consider the question first, but I do think it's a required compatibility improvement before we move ahead with this. |
Well, I certainly had not considered people would be using I thought this was easier to grasp than my first idea: e.g., tag[-2:] in I'll start counting hypens, but am also curious about what is good/bad Many thanks for the review! On 12/8/2019 1:45 PM, Nick Coghlan wrote:
|
Thanks for your patience Michael! I made some cosmetic changes to the error handling logic that you may want to include in the PyPA patches. (I'd intended to make it so that a malformed build date resulted in the "Unknown" "9898" build date being used, rather than triggering a ValueError on import of the AIX support module, but my except clause was too narrow. A PR to widen it to trap ValueError as well would definitely be acceptable). For detection of tag variants, counting hyphens has the benefit of being able to distinguish more variants than just two, but checking the final field would indeed work for distinguishing the current two formats. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: