From 40904e3a058e5aca33f7103bd7769b98242c18fa Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 25 Oct 2020 10:35:38 +0000 Subject: [PATCH 1/5] Remove --build-dir option, as per deprecation --- src/pip/_internal/cli/base_command.py | 14 -------------- src/pip/_internal/cli/cmdoptions.py | 23 ----------------------- src/pip/_internal/commands/download.py | 5 +---- src/pip/_internal/commands/install.py | 6 +----- src/pip/_internal/commands/wheel.py | 5 +---- src/pip/_internal/req/req_install.py | 4 ++++ src/pip/_internal/utils/temp_dir.py | 2 ++ 7 files changed, 9 insertions(+), 50 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index e4b07e0ce8c..f4f86e0e0d0 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/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 " diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py index 2f640b2cbb2..e96eac586db 100644 --- a/src/pip/_internal/cli/cmdoptions.py +++ b/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', diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py index 2f151e049cf..9535ef3cbe4 100644 --- a/src/pip/_internal/commands/download.py +++ b/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, ) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index e41660070a0..a4001553bfa 100644 --- a/src/pip/_internal/commands/install.py +++ b/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, ) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 8f5783c353f..2d654338d7a 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/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, ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 8ce299503b8..866d18fcb6e 100644 --- a/src/pip/_internal/req/req_install.py +++ b/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) diff --git a/src/pip/_internal/utils/temp_dir.py b/src/pip/_internal/utils/temp_dir.py index 03aa8286670..220a093e2fe 100644 --- a/src/pip/_internal/utils/temp_dir.py +++ b/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) From 97e2c7a345c9d626eae0f37f7611775ae974ed5b Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 25 Oct 2020 10:38:18 +0000 Subject: [PATCH 2/5] Add news entry --- news/9049.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/9049.feature.rst diff --git a/news/9049.feature.rst b/news/9049.feature.rst new file mode 100644 index 00000000000..1cf0916f9a2 --- /dev/null +++ b/news/9049.feature.rst @@ -0,0 +1 @@ +Remove the ``--build-dir`` option, as per the deprecation. From 7c4c5b83300a44c506ab4f93a12080a34d4b2705 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 25 Oct 2020 14:26:28 +0000 Subject: [PATCH 3/5] Remove test_pip_wheel_fail_cause_of_previous_build_dir as it's no longer valid --- tests/functional/test_wheel.py | 38 +--------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py index 0b58c923785..73c741f8d2d 100644 --- a/tests/functional/test_wheel.py +++ b/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 @@ -229,42 +229,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.""" From 0c2a2bc803b677e9b5a4b0cc571b7ed070da4e78 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 25 Oct 2020 19:06:14 +0000 Subject: [PATCH 4/5] Mark a couple of tests that still use --build as xfail --- tests/functional/test_install_cleanup.py | 3 +++ tests/functional/test_wheel.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/functional/test_install_cleanup.py b/tests/functional/test_install_cleanup.py index c01c47c3e3e..30102fd2096 100644 --- a/tests/functional/test_install_cleanup.py +++ b/tests/functional/test_install_cleanup.py @@ -7,6 +7,9 @@ @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 diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py index 73c741f8d2d..5e91fea8ab8 100644 --- a/tests/functional/test_wheel.py +++ b/tests/functional/test_wheel.py @@ -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, From 52e1c9a39c363d7443f001c375e7eb2705f5605a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 27 Oct 2020 15:44:17 +0000 Subject: [PATCH 5/5] Not clear why this didn't fail before... --- tests/functional/test_install_cleanup.py | 35 ------------------------ 1 file changed, 35 deletions(-) diff --git a/tests/functional/test_install_cleanup.py b/tests/functional/test_install_cleanup.py index 30102fd2096..10e49124960 100644 --- a/tests/functional/test_install_cleanup.py +++ b/tests/functional/test_install_cleanup.py @@ -1,10 +1,7 @@ -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( @@ -26,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"""