The Wayback Machine - https://web.archive.org/web/20220203065127/https://github.com/python/cpython/pull/28235
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

Jump to bottom

bpo-44860: Update test_sysconfig for posix_user platlib #28235

Merged
merged 3 commits into from Sep 9, 2021
Merged

bpo-44860: Update test_sysconfig for posix_user platlib #28235

merged 3 commits into from Sep 9, 2021

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Sep 8, 2021 •
edited by bedevere-bot

Update test_sysconfig.test_user_similar() for the posix_user scheme:
"platlib" doesn't use sys.platlibdir.

https://bugs.python.org/issue44860
@vstinner
bpo-44860: Update test_sysconfig for posix_user platlib
91d645e 
Update test_sysconfig.test_user_similar() for the posix_user scheme:
"platlib" doesn't use sys.platlibdir.
@vstinner
Copy link
Member Author

@vstinner vstinner commented Sep 8, 2021

@uranusjr: Would you mind to review my change?

cc @hroncok @encukou @frenzymadness
hroncok
hroncok reviewed Sep 8, 2021
View changes
Lib/test/test_sysconfig.py Outdated
self.assertEqual(user_path, global_path.replace(base, user, 1))
expected = global_path.replace(base, user, 1)
# bpo-44860: platlib of posix_user doesn't use sys.platlibdir
if name == 'platlib' and sys.platlibdir != 'lib':
Copy link
Contributor

@hroncok hroncok Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this? The replacement can always happen.

Suggested change
if name == 'platlib' and sys.platlibdir != 'lib':
if name == 'platlib':
Copy link
Member Author

@vstinner vstinner Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str.replace() is not reliabe, the full path may contain the /lib/ substring multiple times. Like /tmp/lib/runtests/lib/python3.9/. I prefer to only use it if it's really needed :-p

Or maybe we should use a more reliable replacement, like a regex?
@hroncok

This comment has been hidden.

Sign in to view
@hroncok

This comment has been hidden.

Sign in to view
hroncok
hroncok approved these changes Sep 8, 2021
View changes
Copy link
Contributor

@hroncok hroncok left a comment •
edited

This fixes the failure for Fedora. Not sure if this test is worth keeping, but that's a question that is out of scope here.
@vstinner
Copy link
Member Author

@vstinner vstinner commented Sep 8, 2021

I'm fine with removing the test: it was added 11 years ago by https://bugs.python.org/issue8759 sysconfig evolved a lot in the meanwhile, and the test became less relevant since sys.platlibdir was added to Python 3.9.
https://docs.python.org/dev/library/sys.html#sys.platlibdir

But I don't know the sysconfig module. In case of doubt, I prefer to minimize changes.
@vstinner vstinner merged commit 49acac0 into python:main Sep 9, 2021
12 checks passed
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 9, 2021

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖
@vstinner vstinner deleted the platlibdir branch Sep 9, 2021
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 9, 2021

Sorry @vstinner, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 49acac00c08838d8080ce00d02c05284b94f8fb2 3.10
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 9, 2021

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Sep 9, 2021

GH-28251 is a backport of this pull request to the 3.10 branch.
miss-islington added a commit to miss-islington/cpython that referenced this issue Sep 9, 2021
@vstinner @miss-islington
bpo-44860: Update test_sysconfig for posix_user platlib (pythonGH-28235)
47f1442 
Update test_sysconfig.test_user_similar() for the posix_user scheme:
"platlib" doesn't use sys.platlibdir.
(cherry picked from commit 49acac0)

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner added a commit that referenced this issue Sep 9, 2021
@vstinner @miss-islington
bpo-44860: Update test_sysconfig for posix_user platlib (GH-28235) (G…
11103eb 
…H-28251)

Update test_sysconfig.test_user_similar() for the posix_user scheme:
"platlib" doesn't use sys.platlibdir.
(cherry picked from commit 49acac0)

Co-authored-by: Victor Stinner <vstinner@python.org>
pablogsal added a commit that referenced this issue Sep 29, 2021
@vstinner @miss-islington @pablogsal
bpo-44860: Update test_sysconfig for posix_user platlib (GH-28235) (G…
01be518 
…H-28251)

Update test_sysconfig.test_user_similar() for the posix_user scheme:
"platlib" doesn't use sys.platlibdir.
(cherry picked from commit 49acac0)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA signed type-tests
5 participants
@vstinner @hroncok @miss-islington @bedevere-bot @the-knights-who-say-ni