Skip to content

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