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.

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

Better platform.processor support #80148

Closed
jaraco opened this issue Feb 11, 2019 · 33 comments
Closed

Better platform.processor support #80148

jaraco opened this issue Feb 11, 2019 · 33 comments
Assignees
Labels
3.9 only security fixes

Comments

@jaraco
Copy link
Member

jaraco commented Feb 11, 2019

BPO 35967
Nosy @malemburg, @gpshead, @jaraco, @pitrou, @miss-islington
PRs
  • bpo-35967: Refreshed platform module with native processor resolvers #12230
  • bpo-35967 resolve platform.processor late #12239
  • bpo-35967: Baseline values for uname -p #12824
  • bpo-35967: Make test_platform.test_uname_processor more lenient to satisfy build bots. #19544
  • bpo-35967: Skip test with uname -p on Android #19577
  • 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:

    assignee = 'https://github.com/jaraco'
    closed_at = <Date 2022-03-20.21:10:53.429>
    created_at = <Date 2019-02-11.14:29:23.506>
    labels = ['3.9']
    title = 'Better platform.processor support'
    updated_at = <Date 2022-03-20.23:42:53.725>
    user = 'https://github.com/jaraco'

    bugs.python.org fields:

    activity = <Date 2022-03-20.23:42:53.725>
    actor = 'yan12125'
    assignee = 'jaraco'
    closed = True
    closed_date = <Date 2022-03-20.21:10:53.429>
    closer = 'jaraco'
    components = []
    creation = <Date 2019-02-11.14:29:23.506>
    creator = 'jaraco'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35967
    keywords = ['patch']
    message_count = 33.0
    messages = ['335220', '335372', '337454', '337475', '337480', '337481', '337483', '337485', '337487', '337488', '337499', '337500', '337503', '337509', '337511', '337513', '337517', '337518', '337519', '338081', '340176', '340209', '366537', '366542', '366543', '366551', '366572', '366594', '366718', '368765', '368803', '369200', '415633']
    nosy_count = 5.0
    nosy_names = ['lemburg', 'gregory.p.smith', 'jaraco', 'pitrou', 'miss-islington']
    pr_nums = ['12230', '12239', '12824', '19544', '19577']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue35967'
    versions = ['Python 3.9']

    @jaraco
    Copy link
    Member Author

    jaraco commented Feb 11, 2019

    or: Unable to implement 'uname' on Python due to recursive call
    or: platform.uname() should avoid calling uname in a subprocess

    In this issue, I stumbled across a strange and somewhat unintuitive behavior. This project attempts to supply a uname executable implemented in Python, so that such functionality could be exposed on any platform including Windows.

    What I found, however, was that because the stdlib platform module actually invokes sh -c uname -p during most any call of the module (

    processor = _syscmd_uname('-p', '')
    ), attempting to use that functionality on a system where uname is implemented by Python (and on the path), will probably fail after a long delay due to infinite recursion.

    Moreover, the only call that's currently invoking uname in a subprocess is the processor resolution, which I suspect is rarely used, in part because the results from it are inconsistent and not particularly useful.

    For example, on Windows, you get a detailed description from the hardware: 'Intel64 Family 6 Model 142 Stepping 9, GenuineIntel'

    On macOS, you get just 'i386'.

    And on Linux, I see 'x86_64' or sometimes just '' (in docker).

    To make matters even worse, this uname -p call happens unconditionally on non-Windows systems for nearly any call in platform. As a result, it's impossible to suppress the invocation of uname, especially when functionality like pkg_resources and its environment markers is invoked early.

    I suggest instead the platform module should (a) resolve processor information in a more uniform manner and (b) not ever call uname, maybe with something like this.

    At the very least, the uname call should be late-bound so that it's not invoked unconditionally for rarely-used information.

    After some period for comment, I'll draft an implementation.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 12, 2019

    Your proposal sounds fine to me. You could fall back on platform.machine() instead of calling uname explicitly.

    @jaraco jaraco added the 3.8 only security fixes label Mar 8, 2019
    @jaraco jaraco self-assigned this Mar 8, 2019
    @malemburg
    Copy link
    Member

    As the documentation says, the API is intended as fairly portable implementation of the Unix uname helper across platforms. It's fine to redirect this directly to e.g. /proc output instead of using the executable, but in whatever you do here, the output of platform.uname() needs to stay compatible to what the function returned prior to such a change, which usually means: to the output of the uname helper on a system.

    Could you please check that on most systems, the output remains the same ?

    Thanks.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    It won't be possible in general to emit what the function returned before, as uname is a symbolic reference to an arbitrary executable, which can vary by platform and release and local environment.

    What I might be able to do is find the implementation of "uname" and see if there's a way to get the value from the same source. I did find what I believe is the canonical source.

    I'll explore if those calls can be translated to Python.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    The first call I see in that routine is to "sysinfo", but the signature of that function doesn't match what I find in the man pages for that function. So that function must be coming from elsewhere.

    @malemburg
    Copy link
    Member

    Thanks. It would be good to do some before/after tests on popular
    platforms, e.g. a few Linuxes, MacOS, Windows.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Aha! It seems the 'sysinfo' call is for Solaris: https://docs.oracle.com/cd/E23823_01/html/816-5167/sysinfo-2.html

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Best I can tell, neither sysinfo nor sysctl are exposed in any way to Python, so it may not be possible to accurately load the processor information from those system calls without writing a wrapper in C. What I might try is to experiment with ctypes to see if I can prove the concept.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Reading further, the 'sysctl' call seems to only be for BSD (https://www.freebsd.org/cgi/man.cgi?sysctl(3)). I could find the man page for sysctl for BSD but not Linux. There is a _sysctl in Linux (http://man7.org/linux/man-pages/man2/sysctl.2.html), but it's use is discouraged and it doesn't provide the necessary information.

    Now I suspect that the aforementioned GNU coreutils 'uname' implementation is only for non-Linux systems, as none of the underlying system calls are relevant on Linux. I expect if one compiled that uname on Linux, 'uname -p' would emit 'unknown'.

    Meaning I still don't know how to get a 'uname -p' result on Linux (without invoking uname -p).

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Hmm. But if I go to the Linux man page for uname (https://linux.die.net/man/1/uname) and follow the links to the source code, I end up at the same repository. So maybe the BSD man page is suitable for Linux. I'll work from that assumption for now.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    After fussing with sysctl for a while, I'm fairly confident that one can't use sysctl on Linux reliably (https://stackoverflow.com/a/55066774/70170). I'll keep digging to see if I can find another implementation of uname that's used on Linux.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    This answer is extremely helpful. uname -p isn't available on Linux except Fedora and late versions of Debian that apply the patch.

    This lack of consistency means that platform.uname().processor and thus platform.processor() is an inherently unreliable interface.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Correction on last comment: s/Debian/Ubuntu/

    @malemburg
    Copy link
    Member

    Jason: StackExchange does have lots of good hints, but it's not always
    the correct. In this case, it's clearly wrong. uname -p has been
    available on many Unix installations for decades.

    I started writing the module back in 1999 and even then, the support
    was already working on the systems I used at the time, and several
    others, as you can see from this page:

    https://www.egenix.com/www2002/python/mxCGIPython.html

    The module was originally created to come up with a good name to
    use for identifying platform binaries coming out of my mxCGIPython
    project.

    Note that the processor is not always needed to determine whether
    software runs on a machine or not. The "uname -m" output often
    is enough, but there are cases where e.g. compiler options are
    used which produces code that only works on particular processors.

    Perhaps adding a more capable API to interface to /proc/cpuinfo
    would be a good idea.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    the output of platform.uname() needs to stay compatible to what the function returned prior

    Do we really wish to retain the output for this unreliable interface, especially when it is not standardized and is returning improper information? Is it valuable for platform.processor() to return "i386" (a 34-year-old processor) for my 2017 Macbook Pro?

    Does maintaining compatibility for platform.uname() also imply that platform.processor() needs to return platform.uname().processor, or could the interface on the latter change, to provide a more useful value, while retaining the behavior of platform.uname()?

    My instinct is it's impractical to attempt to maintain all of these forks of "uname -p", especially when the result is a largely unpredictable value, so I'm considering the only other viable option I can conceive now:

    • retain the subprocess call to "uname", but bind it late, as a functools.cached_property, such that "uname -p" is only ever called when the processor property is requested. This approach would also require overriding __iter__ and __getitem__ to retain the namedtuple interface while having that element resolved late.

    I was also considering this: instead of invoking "uname" anywhere on the path, invoke it from an explicit whitelist of paths, such as /bin and /usr/bin, so that it's never self-referential. Unfortunately, that wouldn't work if a Python-based implementation were put on one of those paths, so it would be brittle at best.

    Marc-Andre, I'd love your feedback in light of these challenges.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    Perhaps adding a more capable API to interface to /proc/cpuinfo
    would be a good idea.

    The core concern I want to address is that it's not possible to use any function in the platform module without invoking "uname -p", and thus it's not possible to implement "uname" in Python. No amount of supplementary interfaces will help with that.

    @malemburg
    Copy link
    Member

    On 08.03.2019 18:00, Jason R. Coombs wrote:

    > Perhaps adding a more capable API to interface to /proc/cpuinfo
    would be a good idea.

    The core concern I want to address is that it's not possible to use any function in the platform module without invoking "uname -p", and thus it's not possible to implement "uname" in Python. No amount of supplementary interfaces will help with that.

    I don't know where you get that idea from. The uname family of APIs
    do use "uname -p" on platforms where this exists, but the other
    ones don't.

    It's also easy to bypass that by simply seeding the global cache
    for uname(): _uname_cache. Or you could call your utility
    something else. Or you could monkey-patch the platform module
    in your utility to work around the circular reference.

    To be clear: I do not consider your use case to be particularly common
    enough to warrant changes to the module, but would welcome additions
    which bring more or better functionality to the module, e.g. having
    the processor variable return meaningful where it previously did
    not (ie. uname() return '' for the processor entry), or adding
    another API to provide more detailed information.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    In this commit, I demonstrate the alternative approach I was considering that avoids calling "uname -p" until it's required, but otherwise retains compatibility by using the same logic for resolving the processor on the various platforms.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 8, 2019

    It's also easy to bypass that by simply seeding the global cache
    for uname(): _uname_cache.
    Or you could monkey-patch the platform module
    in your utility to work around the circular reference.

    I don't think these options are possible in the general case. It was what I attempted to do in the first place, but could not. Consider the situation where a namespace package is present or where a script uses pkg_resources to bootstrap itself (a very common case), or any other case where platform.(anything) is invoked before the "bypass" or "monkey-patch" has a chance to run. This happens when running the test suite for cmdix because pytest invokes pkg_resources to search for entry points and that code invokes platform.system (or similar) to evaluate environment markers long before the cmdix code has been imported.

    Here's what happens:

    platform.(anything) runs platform.uname and platform.uname invokes uname -p in a subprocess unconditionally. Python doesn't provide hooks to monkey-patch that out before it gets invoked.

    Or you could call your utility something else.

    The point of this utility is to supply "coreutils" using Python. It's derived from an abandoned project called "pycoreutils", one purpose of which is to provide the core utilities on a minimal Linux distribution that doesn't have uname. Another is to supply coreutils on Windows. Having an alternate name isn't really viable when the purpose is to supply that interface.

    I do think your considerations are reasonable, and I'm close to giving up. I look forward to your feedback on the 'resolved-late' branch.

    @malemburg
    Copy link
    Member

    On 08.03.2019 18:50, Jason R. Coombs wrote:

    It's also easy to bypass that by simply seeding the global cache
    for uname(): _uname_cache.
    Or you could monkey-patch the platform module
    in your utility to work around the circular reference.

    I don't think these options are possible in the general case. It was what I attempted to do in the first place, but could not. Consider the situation where a namespace package is present or where a script uses pkg_resources to bootstrap itself (a very common case), or any other case where platform.(anything) is invoked before the "bypass" or "monkey-patch" has a chance to run. This happens when running the test suite for cmdix because pytest invokes pkg_resources to search for entry points and that code invokes platform.system (or similar) to evaluate environment markers long before the cmdix code has been imported.

    I don't quite follow: since you are the author of the tool, you can of
    course have your uname.py import platform and then apply one of the
    above tricks, e.g.

    """
    #!/usr/bin/env python3
    import platform

    # Seed uname cache to avoid calling uname
    platform._uname_cache = platform.uname_result(
        system='Linux',
        node='moon',
        release='5.99.99',
        version='#1 SMP 2020',
        machine='x86_64',
        processor='x86_64')

    print ('Hello from uname.py')
    print ('platform.uname() = %r' % (platform.uname(),))
    """

    Here's what happens:

    platform.(anything) runs platform.uname and platform.uname invokes uname -p in a subprocess unconditionally. Python doesn't provide hooks to monkey-patch that out before it gets invoked.

    This is only true for the platform APIs which need information from
    uname. Not in general.

    > Or you could call your utility something else.

    The point of this utility is to supply "coreutils" using Python. It's derived from an abandoned project called "pycoreutils", one purpose of which is to provide the core utilities on a minimal Linux distribution that doesn't have uname. Another is to supply coreutils on Windows. Having an alternate name isn't really viable when the purpose is to supply that interface.

    I do think your considerations are reasonable, and I'm close to giving up. I look forward to your feedback on the 'resolved-late' branch.

    I don't have anything against making calling of uname lazy.
    I also don't have anything against return useful information
    rather than "unknown".

    Your PR is missing tests, though, to support that it actually
    returns the same values are before for a set of common platforms.

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 14, 2019

    I don't quite follow: since you are the author of the tool, you can of
    course have your uname.py import platform and then apply one of the
    above tricks.

    I thought I'd tried that, but failed ref, which is why I committed this change.

    The problem is that, if pkg_resources is used to implement the entry point for the uname console script, or if any other library happens to call platform.*, such as in site.py, before the patch has been allowed to run, the invocation of uname itself ends up invoking uname, causing unlimited recursion. No amount of patching in the uname command implementation can help that.

    Your PR is missing tests, though, to support that it actually
    returns the same values are before for a set of common platforms.

    Yes, that sounds like a good plan. I'll add some tests that assert the values and then update the tests to match the current output, establish a baseline.

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 14, 2019

    In PR 12824 (#12824), I've developed a test that should assure the current output from uname().processor.

    I've merged those changes with PR 12239, which if the tests pass, should illustrate the values returned are unchanged.

    @lemburg, would you be willing to review these PRs to confirm they capture and address your concern?

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 15, 2020

    New changeset 4b4e90a by Jason R. Coombs in branch 'master':
    bpo-35967: Baseline values for uname -p (GH-12824)
    4b4e90a

    @jaraco jaraco added 3.9 only security fixes and removed 3.8 only security fixes labels Apr 15, 2020
    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 15, 2020

    The aformentioned test broke tests in buildbots: https://buildbot.python.org/all/#builders/105/builds/779

    @gpshead
    Copy link
    Member

    gpshead commented Apr 15, 2020

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 15, 2020

    I'm hoping that PR 19544 fixes the issue.

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 15, 2020

    New changeset e72cbcb by Jason R. Coombs in branch 'master':
    bpo-35967: Make test_platform.test_uname_processor more lenient to satisfy build bots. (GH-19544)
    e72cbcb

    @jaraco
    Copy link
    Member Author

    jaraco commented Apr 16, 2020

    New changeset 518835f by Jason R. Coombs in branch 'master':
    bpo-35967 resolve platform.processor late (GH-12239)
    518835f

    @jaraco jaraco closed this as completed Apr 16, 2020
    @miss-islington
    Copy link
    Contributor

    New changeset fb94040 by Chih-Hsuan Yen in branch 'master':
    bpo-35967: Skip test with uname -p on Android (GH-19577)
    fb94040

    @malemburg
    Copy link
    Member

    Reopening the ticket, since the implementation makes backwards incompatible changes to platform.uname(): see https://bugs.python.org/issue40570 for a discussion on a better approach to lazy evaluation of getting the processor information.

    Before we head on into implementation details, could you please point me to the motivation why only the processor detail of uname() needs lazy evaluation ?

    Thanks.

    @malemburg malemburg reopened this May 13, 2020
    @jaraco
    Copy link
    Member Author

    jaraco commented May 13, 2020

    My bad. I probably could have been more proactive about providing a reproducer. The problem, as described above (msg335220) and in the associated cmdix ticket, is that invocation of platform.(anything) causes shelling out to execute "uname", so it's not possible to implement uname on Python unless one can guarantee that platform.(anything) is not invoked prior to the Python uname implementation executing.

    Here's a Dockerfile replicating the issue:

    FROM ubuntu:focal
    
    # Install Python
    RUN apt update
    RUN apt install -y python3
    RUN ln -s /usr/bin/python3 /usr/bin/python
    
    # Simulate something on the system that invokes platform early.
    RUN printf 'print(__import__("platform").system())' > sitecustomize.py
    ENV PYTHONPATH=/
    
    # Create a stubbed 'uname' command build in Python
    ENV PATH=/:$PATH
    RUN printf '#!/usr/bin/env python\nprint("getting ready to patch platform", flush=True)' > uname
    RUN chmod u+x uname
    
    CMD uname
    

    As you can see, this reproducer creates a very simple 'uname' implementation. All it does is print that it's about to patch the platform module (because maybe that will make things work). Unfortunately, that behavior is never reached because before that code has a chance to run, sitecustomize is imported and calls platform.system(), which invokes platform.uname() which attempts to resolve the processor, which attempts to invoke uname -p (even on Windows), which invokes the stubbed uname command, and infinite recursion begins.

    The sitecustomize might seem a little contrived, except that a very similar behavior occurs in a very typical environment:

    • pip, when installing a package for editing, invokes setuptools to develop the package.
    • setuptools, when installing a package for developing, creates command-line entry points using a routine that imports pkg_resources to ensure that the relevant packages are present before invoking the command's functionality.
    • pkg_resources imports packaging to evaluate markers.
    • packaging uses platform.system() and other behaviors to evaluate the markers.

    So ultimately, the same behavior is triggered before the user's code is ever executed.

    But more importantly, why should "uname -p" be invoked in a subprocess on Windows to get "platform.system()"?

    @malemburg
    Copy link
    Member

    Thanks, Jason. I'll have a closer look at the issue and report back later this week.

    @jaraco
    Copy link
    Member Author

    jaraco commented Mar 20, 2022

    I'm going to close this issue again, as the implementation is now present in at least a couple of releases. May I suggest that if there are ongoing concerns or issues to open up a new issue and reference this one and loop me into the conversation? I'm also happy to re-open this one as well.

    @jaraco jaraco closed this as completed Mar 20, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants