Skip to content

Globally manage and track some temp build directories #7677

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

Merged
merged 8 commits into from
Feb 5, 2020
14 changes: 6 additions & 8 deletions src/pip/_internal/build_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from pip import __file__ as pip_location
from pip._internal.utils.subprocess import call_subprocess
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.ui import open_spinner

Expand Down Expand Up @@ -54,10 +54,12 @@ class BuildEnvironment(object):

def __init__(self):
# type: () -> None
self._temp_dir = TempDirectory(kind="build-env")
temp_dir = TempDirectory(
kind=tempdir_kinds.BUILD_ENV, globally_managed=True
)

self._prefixes = OrderedDict((
(name, _Prefix(os.path.join(self._temp_dir.path, name)))
(name, _Prefix(os.path.join(temp_dir.path, name)))
for name in ('normal', 'overlay')
))

Expand All @@ -76,7 +78,7 @@ def __init__(self):
get_python_lib(plat_specific=True),
)
}
self._site_dir = os.path.join(self._temp_dir.path, 'site')
self._site_dir = os.path.join(temp_dir.path, 'site')
if not os.path.exists(self._site_dir):
os.mkdir(self._site_dir)
with open(os.path.join(self._site_dir, 'sitecustomize.py'), 'w') as fp:
Expand Down Expand Up @@ -133,10 +135,6 @@ def __exit__(self, exc_type, exc_val, exc_tb):
else:
os.environ[varname] = old_value

def cleanup(self):
# type: () -> None
self._temp_dir.cleanup()

def check_requirements(self, reqs):
# type: (Iterable[str]) -> Tuple[Set[Tuple[str, str]], Set[str]]
"""Return 2 sets:
Expand Down
50 changes: 47 additions & 3 deletions src/pip/_internal/cli/req_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
import os
from functools import partial

from pip._internal.cli import cmdoptions
from pip._internal.cli.base_command import Command
from pip._internal.cli.command_context import CommandContextMixIn
from pip._internal.exceptions import CommandError
from pip._internal.exceptions import CommandError, PreviousBuildDirError
from pip._internal.index.package_finder import PackageFinder
from pip._internal.legacy_resolve import Resolver
from pip._internal.models.selection_prefs import SelectionPreferences
Expand All @@ -28,16 +29,22 @@
make_link_collector,
pip_self_version_check,
)
from pip._internal.utils.temp_dir import tempdir_kinds
from pip._internal.utils.typing import MYPY_CHECK_RUNNING

if MYPY_CHECK_RUNNING:
from optparse import Values
from typing import List, Optional, Tuple
from typing import Any, List, Optional, Tuple

from pip._internal.cache import WheelCache
from pip._internal.models.target_python import TargetPython
from pip._internal.req.req_set import RequirementSet
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.temp_dir import (
TempDirectory,
TempDirectoryTypeRegistry,
)


logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -149,8 +156,45 @@ def handle_pip_version_check(self, options):
pip_self_version_check(session, options)


KEEPABLE_TEMPDIR_TYPES = [tempdir_kinds.BUILD_ENV, tempdir_kinds.REQ_BUILD]


def with_cleanup(func):
# type: (Any) -> Any
"""Decorator for common logic related to managing temporary
directories.
"""
def configure_tempdir_registry(registry):
# type: (TempDirectoryTypeRegistry) -> None
for t in KEEPABLE_TEMPDIR_TYPES:
registry.set_delete(t, False)

def wrapper(self, options, args):
# type: (RequirementCommand, Values, List[Any]) -> Optional[int]
assert self.tempdir_registry is not None
if options.no_clean:
configure_tempdir_registry(self.tempdir_registry)

try:
return func(self, options, args)
except PreviousBuildDirError:
# This kind of conflict can occur when the user passes an explicit
# build directory with a pre-existing folder. In that case we do
# not want to accidentally remove it.
configure_tempdir_registry(self.tempdir_registry)
raise

return wrapper


class RequirementCommand(IndexGroupCommand):

def __init__(self, *args, **kw):
# type: (Any, Any) -> None
super(RequirementCommand, self).__init__(*args, **kw)

self.cmd_opts.add_option(cmdoptions.no_clean())

@staticmethod
def make_requirement_preparer(
temp_build_dir, # type: TempDirectory
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from pip._internal.cli import cmdoptions
from pip._internal.cli.cmdoptions import make_target_python
from pip._internal.cli.req_command import RequirementCommand
from pip._internal.cli.req_command import RequirementCommand, with_cleanup
from pip._internal.req import RequirementSet
from pip._internal.req.req_tracker import get_requirement_tracker
from pip._internal.utils.misc import ensure_dir, normalize_path, write_output
Expand Down Expand Up @@ -52,7 +52,6 @@ def __init__(self, *args, **kw):
cmd_opts.add_option(cmdoptions.prefer_binary())
cmd_opts.add_option(cmdoptions.src())
cmd_opts.add_option(cmdoptions.pre())
cmd_opts.add_option(cmdoptions.no_clean())
cmd_opts.add_option(cmdoptions.require_hashes())
cmd_opts.add_option(cmdoptions.progress_bar())
cmd_opts.add_option(cmdoptions.no_build_isolation())
Expand All @@ -77,6 +76,7 @@ def __init__(self, *args, **kw):
self.parser.insert_option_group(0, index_opts)
self.parser.insert_option_group(0, cmd_opts)

@with_cleanup
def run(self, options, args):
options.ignore_installed = True
# editable doesn't really make sense for `pip download`, but the bowels
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from pip._internal.cache import WheelCache
from pip._internal.cli import cmdoptions
from pip._internal.cli.cmdoptions import make_target_python
from pip._internal.cli.req_command import RequirementCommand
from pip._internal.cli.req_command import RequirementCommand, with_cleanup
from pip._internal.cli.status_codes import ERROR, SUCCESS
from pip._internal.exceptions import (
CommandError,
Expand Down Expand Up @@ -227,7 +227,6 @@ def __init__(self, *args, **kw):
cmd_opts.add_option(cmdoptions.no_binary())
cmd_opts.add_option(cmdoptions.only_binary())
cmd_opts.add_option(cmdoptions.prefer_binary())
cmd_opts.add_option(cmdoptions.no_clean())
cmd_opts.add_option(cmdoptions.require_hashes())
cmd_opts.add_option(cmdoptions.progress_bar())

Expand All @@ -239,6 +238,7 @@ def __init__(self, *args, **kw):
self.parser.insert_option_group(0, index_opts)
self.parser.insert_option_group(0, cmd_opts)

@with_cleanup
def run(self, options, args):
# type: (Values, List[Any]) -> int
cmdoptions.check_install_build_global(options)
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from pip._internal.cache import WheelCache
from pip._internal.cli import cmdoptions
from pip._internal.cli.req_command import RequirementCommand
from pip._internal.cli.req_command import RequirementCommand, with_cleanup
from pip._internal.exceptions import CommandError, PreviousBuildDirError
from pip._internal.req import RequirementSet
from pip._internal.req.req_tracker import get_requirement_tracker
Expand Down Expand Up @@ -101,7 +101,6 @@ def __init__(self, *args, **kw):
"pip only finds stable versions."),
)

cmd_opts.add_option(cmdoptions.no_clean())
cmd_opts.add_option(cmdoptions.require_hashes())

index_opts = cmdoptions.make_option_group(
Expand All @@ -112,6 +111,7 @@ def __init__(self, *args, **kw):
self.parser.insert_option_group(0, index_opts)
self.parser.insert_option_group(0, cmd_opts)

@with_cleanup
def run(self, options, args):
# type: (Values, List[Any]) -> None
cmdoptions.check_install_build_global(options)
Expand Down
11 changes: 5 additions & 6 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
rmtree,
)
from pip._internal.utils.packaging import get_metadata
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.virtualenv import running_under_virtualenv
from pip._internal.vcs import vcs
Expand Down Expand Up @@ -358,7 +358,9 @@ def ensure_build_location(self, build_dir):
# Some systems have /tmp as a symlink which confuses custom
# builds (such as numpy). Thus, we ensure that the real path
# is returned.
self._temp_build_dir = TempDirectory(kind="req-build")
self._temp_build_dir = TempDirectory(
kind=tempdir_kinds.REQ_BUILD, globally_managed=True
)

return self._temp_build_dir.path
if self.editable:
Expand Down Expand Up @@ -418,10 +420,7 @@ def remove_temporary_source(self):
logger.debug('Removing source in %s', self.source_dir)
rmtree(self.source_dir)
self.source_dir = None
if self._temp_build_dir:
self._temp_build_dir.cleanup()
self._temp_build_dir = None
self.build_env.cleanup()
self._temp_build_dir = None

def check_if_exists(self, use_user_site):
# type: (bool) -> None
Expand Down
10 changes: 9 additions & 1 deletion src/pip/_internal/utils/temp_dir.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from pip._vendor.contextlib2 import ExitStack

from pip._internal.utils.misc import rmtree
from pip._internal.utils.misc import enum, rmtree
from pip._internal.utils.typing import MYPY_CHECK_RUNNING

if MYPY_CHECK_RUNNING:
Expand All @@ -21,6 +21,14 @@
logger = logging.getLogger(__name__)


# Kinds of temporary directories. Only needed for ones that are
# globally-managed.
tempdir_kinds = enum(
BUILD_ENV="build-env",
REQ_BUILD="req-build",
)


_tempdir_manager = None # type: Optional[ExitStack]


Expand Down
17 changes: 10 additions & 7 deletions tests/unit/test_build_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def run_with_build_env(script, setup_script_contents,
SelectionPreferences
)
from pip._internal.network.session import PipSession
from pip._internal.utils.temp_dir import global_tempdir_manager

link_collector = LinkCollector(
session=PipSession(),
Expand All @@ -41,19 +42,21 @@ def run_with_build_env(script, setup_script_contents,
link_collector=link_collector,
selection_prefs=selection_prefs,
)
build_env = BuildEnvironment()

try:
with global_tempdir_manager():
build_env = BuildEnvironment()
''' % str(script.scratch_path)) +
indent(dedent(setup_script_contents), ' ') +
dedent(
'''
indent(
dedent(
'''
if len(sys.argv) > 1:
with build_env:
subprocess.check_call((sys.executable, sys.argv[1]))
finally:
build_env.cleanup()
''')
'''
),
' '
)
)
args = ['python', build_env_script]
if test_script_contents is not None:
Expand Down