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

Prevent distutils option error target prefix conflict #6008

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Nov 12, 2018

Fixes #4106

  • Add news fragment

@benoit-pierre
Copy link
Member

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)

@jaraco
Copy link
Member Author

jaraco commented Nov 12, 2018

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!

@jaraco jaraco force-pushed the bugfix/4106-distutils-option-error-target-prefix-conflict branch from 7ae78af to 3f01334 Compare November 12, 2018 18:10
@benoit-pierre
Copy link
Member

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)))

@benoit-pierre
Copy link
Member

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)

@jaraco jaraco force-pushed the bugfix/4106-distutils-option-error-target-prefix-conflict branch from d3837d6 to da9caa4 Compare November 12, 2018 20:46
@jaraco jaraco closed this Nov 12, 2018
@jaraco jaraco reopened this Nov 12, 2018
@rouge8
Copy link
Contributor

rouge8 commented Apr 12, 2019

I merged this with the latest master and tested it locally and it worked! 🎉 Is there any way to get this reviewed and accepted?

rouge8 added a commit to rouge8/dotfiles that referenced this pull request May 14, 2019
@jaraco jaraco requested a review from pradyunsg May 23, 2019 11:16
@pradyunsg pradyunsg merged commit 287aa4b into pypa:master May 25, 2019
@pradyunsg
Copy link
Member

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.

rouge8 added a commit to rouge8/dotfiles that referenced this pull request Jun 5, 2019
Now that pypa/pip#6008 is fixed, shiv works with
a Homebrew Python without any dumb hacks with `~/.pydistutils.cfg`.
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2019
@jaraco jaraco deleted the bugfix/4106-distutils-option-error-target-prefix-conflict branch May 1, 2020 22:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DistutilsOptionError with --target and prefix or exec-prefix
4 participants