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

bpo-28009: Fix SkipUnless logic to be based on platform programs capable of introspection #12777

Merged
merged 7 commits into from
Jun 15, 2019

Conversation

aixtools
Copy link
Contributor

@aixtools aixtools commented Apr 11, 2019

Thanks for a suggestion - for a better way @ncoghlan

Rather than try and get everything in one PR - a mountain to steep for me - I am going to take what I hope is the most simple part of my previous PR's on this PR8672 - still open and PR5183 (which I closed and started over again).

I will continue on PR8672 - but there the focus will be on Lib/uuid.py rather than Lib/test/test_uuid.py

I expect comments:
a) I am using platform.system() rather than sys.platform(.startswith)
b) I have identified four specific platforms (AIX, Darwin, Linux, Windows). Maybe more is needed, or a different default definition for _NODE_GETTERS when not a recognized platform.
c) prepending either _unix_getnode, or windll_getnode to NODE_GETTERS rather than repeat it several times. Not sure what is most "PEP8" specification.

d) In the class BaseTestInternals I removed the statement "uuid = None", better replaced it with _uuid = py_uuid so that the skipUnless statements could find what they need. Maybe "uuid = None" is still needed. However, it seems to unreferenced (as self.uuid is used for the selection of internal or external uuid module actually called)

In the hope this can move forward in steps - the patch focusing on SkipUnless - and getting the real commands that need to be tested right (in NODE_GETTERS).

https://bugs.python.org/issue28009

Lib/uuid.py Outdated Show resolved Hide resolved
@aixtools
Copy link
Contributor Author

FYI: one of the reasons I split this out - to address existing test issues only - is because a slightly hardened AIX server (using AIX standard tools) fails when the arp command is not SUID. So, in order to also run on a hardened server, e.g., as a bot that is not root, this change is actually high impact (for AIX).

@aixtools
Copy link
Contributor Author

p.s. This is a real "security" issue - in that when the tests run on a hardened AIX server (e.g., using aixpert or PowerSC) python consumes lots of resources, stalling the server, and ultimately core dumps. To prevent this on my bot I had to "undo" some of the hardening.

If there are Python concerns with this PR - please comment so that those concerns may be resolved and the PR becomes acceptable. Thx!

@aixtools
Copy link
Contributor Author

Just a reminder - I made this PR because my bot was failing miserably after I had performed some basic system hardening. Without it the system will hang until python (test_uuid) crashes.

Lib/uuid.py Outdated Show resolved Hide resolved
@ncoghlan ncoghlan merged commit 3a1d50e into python:master Jun 15, 2019
@miss-islington
Copy link
Contributor

Thanks @aixtools for the PR, and @ncoghlan for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 15, 2019
… capable of introspection (pythonGH-12777)

uuid could try fallback methods that had no chance of working on a particular
platform, and this could cause spurious test failures, as well as degraded
performance as fallback options were tried and failed.

This fixes both the uuid module and its test's SkipUnless logic to use a
prefiltered list of techniques that may at least potentially work on that platform.

Patch by Michael Felt (aixtools).
(cherry picked from commit 3a1d50e)

Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
@bedevere-bot
Copy link

GH-14115 is a backport of this pull request to the 3.8 branch.

@ncoghlan
Copy link
Contributor

Thanks for the change and the updated comments - they make it a lot clearer what is going on :)

miss-islington added a commit that referenced this pull request Jun 15, 2019
… capable of introspection (GH-12777)

uuid could try fallback methods that had no chance of working on a particular
platform, and this could cause spurious test failures, as well as degraded
performance as fallback options were tried and failed.

This fixes both the uuid module and its test's SkipUnless logic to use a
prefiltered list of techniques that may at least potentially work on that platform.

Patch by Michael Felt (aixtools).
(cherry picked from commit 3a1d50e)

Co-authored-by: Michael Felt <aixtools@users.noreply.github.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
… capable of introspection (pythonGH-12777)

uuid could try fallback methods that had no chance of working on a particular
platform, and this could cause spurious test failures, as well as degraded
performance as fallback options were tried and failed.

This fixes both the uuid module and its test's SkipUnless logic to use a
prefiltered list of techniques that may at least potentially work on that platform.

Patch by Michael Felt (aixtools).
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
… capable of introspection (pythonGH-12777)

uuid could try fallback methods that had no chance of working on a particular
platform, and this could cause spurious test failures, as well as degraded
performance as fallback options were tried and failed.

This fixes both the uuid module and its test's SkipUnless logic to use a
prefiltered list of techniques that may at least potentially work on that platform.

Patch by Michael Felt (aixtools).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants