-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Prevent distutils option error target prefix conflict #6008
Prevent distutils option error target prefix conflict #6008
Conversation
…nvironment and --target is used. Fixes pypa#4106.
Here is a test: tests/functional/test_install.py | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git i/tests/functional/test_install.py w/tests/functional/test_install.py
index 40504a18..d93d68c7 100644
--- i/tests/functional/test_install.py
+++ w/tests/functional/test_install.py
@@ -1444,3 +1444,19 @@ def test_install_conflict_warning_can_be_suppressed(script, data):
'install', '--no-index', pkgB_path, '--no-warn-conflicts'
)
assert "Successfully installed pkgB-2.0" in result2.stdout, str(result2)
+
+
+def test_target_install_ignores_distutils_config_install_prefix(script):
+ prefix = script.scratch_path / 'prefix'
+ Path(os.environ['HOME'], '.pydistutils.cfg').write(textwrap.dedent(
+ '''
+ [install]
+ prefix=%s
+ ''' % str(prefix)))
+ target = script.scratch_path / 'target'
+ result = script.pip_install_local('simplewheel', '-t', target)
+ assert (
+ "Successfully installed simplewheel" in result.stdout and
+ (target - script.base_path) in result.files_created and
+ (prefix - script.base_path) not in result.files_created
+ ), str(result) |
I've added the test to the PR. The main reason I didn't add the test is that it's much more difficult to test the reported failure, which is that prefix is configured globally in distutils.cfg, and I found that simply creating a virtualenv works around the issue, so it may prove very difficult to create a suitable environment for replicating that failure mode. But if this test does at least capture one failure mode, that's much better than no test. Thanks! |
7ae78af
to
3f01334
Compare
Damn it! The test is failing on Windows, should be using this instead for the distutils config location: Path(os.path.expanduser('~'),
'pydistutils.cfg' if sys.platform == 'win32' else '.pydistutils.cfg'
).write(textwrap.dedent(
'''
[install]
prefix=%s
''' % str(prefix))) |
Which of course the linter is going to complain about... Stupid mindless PEP 8 enforcement. def test_target_install_ignores_distutils_config_install_prefix(script):
prefix = script.scratch_path / 'prefix'
distutils_config = Path(os.path.expanduser('~'),
'pydistutils.cfg' if sys.platform == 'win32'
else '.pydistutils.cfg')
distutils_config.write(textwrap.dedent(
'''
[install]
prefix=%s
''' % str(prefix)))
target = script.scratch_path / 'target'
result = script.pip_install_local('simplewheel', '-t', target)
assert (
"Successfully installed simplewheel" in result.stdout and
(target - script.base_path) in result.files_created and
(prefix - script.base_path) not in result.files_created
), str(result) |
d3837d6
to
da9caa4
Compare
I merged this with the latest master and tested it locally and it worked! 🎉 Is there any way to get this reviewed and accepted? |
Until pypa/pip#6008 is merged...
Thanks @benoit-pierre and @jaraco for figuring this out! I'm not super familiar with these details though, so maybe it'd break something else somewhere in an esoteric manner. I don't think that's a likely outcome here though. |
Now that pypa/pip#6008 is fixed, shiv works with a Homebrew Python without any dumb hacks with `~/.pydistutils.cfg`.
Fixes #4106