Merge pull request #9049 from pfmoore/remove_build_dir

Author: pradyunsg

news/9049.feature.rst

@@ -0,0 +1 @@
+Remove the ``--build-dir`` option, as per the deprecation.

src/pip/_internal/cli/base_command.py

@@ -197,20 +197,6 @@ def _main(self, args):
                 )
                 options.cache_dir = None
 
-        if getattr(options, "build_dir", None):
-            deprecated(
-                reason=(
-                    "The -b/--build/--build-dir/--build-directory "
-                    "option is deprecated."
-                ),
-                replacement=(
-                    "use the TMPDIR/TEMP/TMP environment variable, "
-                    "possibly combined with --no-clean"
-                ),
-                gone_in="20.3",
-                issue=8333,
-            )
-
         if 'resolver' in options.unstable_features:
             logger.critical(
                 "--unstable-feature=resolver is no longer supported, and "

src/pip/_internal/cli/cmdoptions.py

@@ -685,29 +685,6 @@ def _handle_no_cache_dir(option, opt, value, parser):
 )  # type: Callable[..., Option]
 
 
-def _handle_build_dir(option, opt, value, parser):
-    # type: (Option, str, str, OptionParser) -> None
-    if value:
-        value = os.path.abspath(value)
-    setattr(parser.values, option.dest, value)
-
-
-build_dir = partial(
-    PipOption,
-    '-b', '--build', '--build-dir', '--build-directory',
-    dest='build_dir',
-    type='path',
-    metavar='dir',
-    action='callback',
-    callback=_handle_build_dir,
-    help='(DEPRECATED) '
-         'Directory to unpack packages into and build in. Note that '
-         'an initial build still takes place in a temporary directory. '
-         'The location of temporary directories can be controlled by setting '
-         'the TMPDIR environment variable (TEMP on Windows) appropriately. '
-         'When passed, build directories are not cleaned in case of failures.'
-)  # type: Callable[..., Option]
-
 ignore_requires_python = partial(
     Option,
     '--ignore-requires-python',

src/pip/_internal/commands/download.py

@@ -43,7 +43,6 @@ def add_options(self):
         # type: () -> None
         self.cmd_opts.add_option(cmdoptions.constraints())
         self.cmd_opts.add_option(cmdoptions.requirements())
-        self.cmd_opts.add_option(cmdoptions.build_dir())
         self.cmd_opts.add_option(cmdoptions.no_deps())
         self.cmd_opts.add_option(cmdoptions.global_options())
         self.cmd_opts.add_option(cmdoptions.no_binary())
@@ -97,13 +96,11 @@ def run(self, options, args):
             session=session,
             target_python=target_python,
         )
-        build_delete = (not (options.no_clean or options.build_dir))
 
         req_tracker = self.enter_context(get_requirement_tracker())
 
         directory = TempDirectory(
-            options.build_dir,
-            delete=build_delete,
+            delete=not options.no_clean,
             kind="download",
             globally_managed=True,
         )

src/pip/_internal/commands/install.py

@@ -129,8 +129,6 @@ def add_options(self):
             help="Installation prefix where lib, bin and other top-level "
                  "folders are placed")
 
-        self.cmd_opts.add_option(cmdoptions.build_dir())
-
         self.cmd_opts.add_option(cmdoptions.src())
 
         self.cmd_opts.add_option(
@@ -277,14 +275,12 @@ def run(self, options, args):
             target_python=target_python,
             ignore_requires_python=options.ignore_requires_python,
         )
-        build_delete = (not (options.no_clean or options.build_dir))
         wheel_cache = WheelCache(options.cache_dir, options.format_control)
 
         req_tracker = self.enter_context(get_requirement_tracker())
 
         directory = TempDirectory(
-            options.build_dir,
-            delete=build_delete,
+            delete=not options.no_clean,
             kind="install",
             globally_managed=True,
         )

src/pip/_internal/commands/wheel.py

@@ -78,7 +78,6 @@ def add_options(self):
         self.cmd_opts.add_option(cmdoptions.src())
         self.cmd_opts.add_option(cmdoptions.ignore_requires_python())
         self.cmd_opts.add_option(cmdoptions.no_deps())
-        self.cmd_opts.add_option(cmdoptions.build_dir())
         self.cmd_opts.add_option(cmdoptions.progress_bar())
 
         self.cmd_opts.add_option(
@@ -115,7 +114,6 @@ def run(self, options, args):
         session = self.get_default_session(options)
 
         finder = self._build_package_finder(options, session)
-        build_delete = (not (options.no_clean or options.build_dir))
         wheel_cache = WheelCache(options.cache_dir, options.format_control)
 
         options.wheel_dir = normalize_path(options.wheel_dir)
@@ -124,8 +122,7 @@ def run(self, options, args):
         req_tracker = self.enter_context(get_requirement_tracker())
 
         directory = TempDirectory(
-            options.build_dir,
-            delete=build_delete,
+            delete=not options.no_clean,
             kind="wheel",
             globally_managed=True,
         )

src/pip/_internal/req/req_install.py

@@ -358,6 +358,10 @@ def ensure_build_location(self, build_dir, autodelete, parallel_builds):
 
             return self._temp_build_dir.path
 
+        # This is the only remaining place where we manually determine the path
+        # for the temporary directory. It is only needed for editables where
+        # it is the value of the --src option.
+
         # When parallel builds are enabled, add a UUID to the build directory
         # name so multiple builds do not interfere with each other.
         dir_name = canonicalize_name(self.name)

src/pip/_internal/utils/temp_dir.py

@@ -134,6 +134,8 @@ def __init__(
                 # tempdir_registry says.
                 delete = None
 
+        # The only time we specify path is in for editables where it
+        # is the value of the --src option.
         if path is None:
             path = self._create(kind)
 

tests/functional/test_install_cleanup.py

@@ -1,12 +1,12 @@
-import os
 from os.path import exists
 
 import pytest
 
-from pip._internal.cli.status_codes import PREVIOUS_BUILD_DIR_ERROR
-
 
 @pytest.mark.network
+@pytest.mark.xfail(
+    reason="The --build option was removed"
+)
 def test_no_clean_option_blocks_cleaning_after_install(script, data):
     """
     Test --no-clean option blocks cleaning after install
@@ -23,38 +23,6 @@ def test_no_clean_option_blocks_cleaning_after_install(script, data):
     assert exists(build)
 
 
-@pytest.mark.network
-def test_cleanup_prevented_upon_build_dir_exception(
-    script,
-    data,
-    use_new_resolver,
-):
-    """
-    Test no cleanup occurs after a PreviousBuildDirError
-    """
-    build = script.venv_path / 'build'
-    build_simple = build / 'simple'
-    os.makedirs(build_simple)
-    build_simple.joinpath("setup.py").write_text("#")
-    result = script.pip(
-        'install', '-f', data.find_links, '--no-index', 'simple',
-        '--build', build,
-        expect_error=(not use_new_resolver),
-        expect_temp=(not use_new_resolver),
-        expect_stderr=True,
-    )
-
-    assert (
-        "The -b/--build/--build-dir/--build-directory "
-        "option is deprecated."
-    ) in result.stderr
-
-    if not use_new_resolver:
-        assert result.returncode == PREVIOUS_BUILD_DIR_ERROR, str(result)
-        assert "pip can't proceed" in result.stderr, str(result)
-        assert exists(build_simple), str(result)
-
-
 @pytest.mark.network
 def test_pep517_no_legacy_cleanup(script, data, with_wheel):
     """Test a PEP 517 failed build does not attempt a legacy cleanup"""

tests/functional/test_wheel.py

@@ -6,7 +6,7 @@
 
 import pytest
 
-from pip._internal.cli.status_codes import ERROR, PREVIOUS_BUILD_DIR_ERROR
+from pip._internal.cli.status_codes import ERROR
 from tests.lib import pyversion  # noqa: F401
 
 
@@ -187,6 +187,9 @@ def test_pip_wheel_fail(script, data):
     assert result.returncode != 0
 
 
+@pytest.mark.xfail(
+    reason="The --build option was removed"
+)
 def test_no_clean_option_blocks_cleaning_after_wheel(
     script,
     data,
@@ -229,42 +232,6 @@ def test_pip_wheel_source_deps(script, data):
     assert "Successfully built source" in result.stdout, result.stdout
 
 
-def test_pip_wheel_fail_cause_of_previous_build_dir(
-    script,
-    data,
-    use_new_resolver,
-):
-    """
-    Test when 'pip wheel' tries to install a package that has a previous build
-    directory
-    """
-
-    # Given that I have a previous build dir of the `simple` package
-    build = script.venv_path / 'build' / 'simple'
-    os.makedirs(build)
-    build.joinpath('setup.py').write_text('#')
-
-    # When I call pip trying to install things again
-    result = script.pip(
-        'wheel', '--no-index',
-        '--find-links={data.find_links}'.format(**locals()),
-        '--build', script.venv_path / 'build',
-        'simple==3.0',
-        expect_error=(not use_new_resolver),
-        expect_temp=(not use_new_resolver),
-        expect_stderr=True,
-    )
-
-    assert (
-        "The -b/--build/--build-dir/--build-directory "
-        "option is deprecated."
-    ) in result.stderr
-
-    # Then I see that the error code is the right one
-    if not use_new_resolver:
-        assert result.returncode == PREVIOUS_BUILD_DIR_ERROR, result
-
-
 def test_wheel_package_with_latin1_setup(script, data):
     """Create a wheel from a package with latin-1 encoded setup.py."""