diff --git a/build-support/bin/release.sh b/build-support/bin/release.sh index e5f58fab474..301ed157746 100755 --- a/build-support/bin/release.sh +++ b/build-support/bin/release.sh @@ -134,11 +134,18 @@ REQUIREMENTS_3RDPARTY_FILES=( # internal backend packages and their dependencies which it doesn't have, # and it'll fail. To solve that problem, we load the internal backend package # dependencies into the pantsbuild.pants venv. +# +# TODO: Starting and stopping pantsd repeatedly here works fine, but because the +# created venvs are located within the buildroot, pantsd will fingerprint them on +# startup. Production usecases should generally not experience this cost, because +# either pexes or venvs (as created by the `pants` script that we distribute) are +# created outside of the buildroot. function execute_packaged_pants_with_internal_backends() { pip install --ignore-installed \ -r pants-plugins/3rdparty/python/requirements.txt &> /dev/null && \ pants \ --no-verify-config \ + --no-enable-pantsd \ --pythonpath="['pants-plugins/src/python']" \ --backend-packages="[\ 'pants.backend.codegen',\ diff --git a/pants.toml b/pants.toml index 8f30030223e..1eebdf8cae7 100644 --- a/pants.toml +++ b/pants.toml @@ -20,6 +20,7 @@ local_artifact_cache = "%(pants_bootstrapdir)s/artifact_cache" [GLOBAL] print_exception_stacktrace = true v2 = false +enable_pantsd = true # Enable our own custom loose-source plugins as well as contribs. pythonpath = [ diff --git a/pants.travis-ci.toml b/pants.travis-ci.toml index 19b19c31e36..d37e52baa6a 100644 --- a/pants.travis-ci.toml +++ b/pants.travis-ci.toml @@ -12,6 +12,10 @@ travis_parallelism = 4 # Override color support detection - we want colors and Travis supports them. colors = true +# TODO: Eventually we would like pantsd enabled in CI as well, but we blanket disable it for +# now in order to smooth off rough edges locally. +enable_pantsd = false + [black] args = ["--quiet"] diff --git a/src/python/pants/bin/BUILD b/src/python/pants/bin/BUILD index 49b56f2a243..e9db54dcfe6 100644 --- a/src/python/pants/bin/BUILD +++ b/src/python/pants/bin/BUILD @@ -8,6 +8,7 @@ python_library( 'src/python/pants/base:build_environment', 'src/python/pants/base:build_file', 'src/python/pants/base:cmd_line_spec_parser', + 'src/python/pants/base:deprecated', 'src/python/pants/base:exception_sink', 'src/python/pants/base:exceptions', 'src/python/pants/base:exiter', diff --git a/src/python/pants/bin/pants_runner.py b/src/python/pants/bin/pants_runner.py index 9d571c737bf..5f9201c4e90 100644 --- a/src/python/pants/bin/pants_runner.py +++ b/src/python/pants/bin/pants_runner.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from typing import List, Mapping +from pants.base.deprecated import deprecated_conditional from pants.base.exception_sink import ExceptionSink from pants.base.exiter import ExitCode from pants.bin.remote_pants_runner import RemotePantsRunner @@ -81,6 +82,18 @@ def run(self, start_time: float) -> ExitCode: global_bootstrap_options.print_exception_stacktrace ) + # TODO: When we remove this deprecation, we'll change the default for the option to true. + deprecated_conditional( + lambda: global_bootstrap_options.is_default("enable_pantsd"), + removal_version="1.30.0.dev0", + entity_description="--enable-pantsd defaulting to False", + hint_message=( + "Pantsd improves runtime performance and will be enabled by default in the 1.30.x " + "stable releases. To prepare for that change, we recommend setting the " + "`[GLOBAL] enable_pantsd` setting to `True` in your pants.toml or pants.ini file." + ), + ) + if self._should_run_with_pantsd(global_bootstrap_options): try: return RemotePantsRunner(self.args, self.env, options_bootstrapper).run(start_time) diff --git a/src/python/pants/init/options_initializer.py b/src/python/pants/init/options_initializer.py index 27589a0ea42..6dfea7eb123 100644 --- a/src/python/pants/init/options_initializer.py +++ b/src/python/pants/init/options_initializer.py @@ -17,6 +17,7 @@ from pants.option.global_options import GlobalOptions from pants.subsystem.subsystem import Subsystem from pants.util.dirutil import fast_relpath_optional +from pants.util.ordered_set import OrderedSet logger = logging.getLogger(__name__) @@ -134,13 +135,15 @@ def compute_pantsd_invalidation_globs(buildroot, bootstrap_options): Combines --pythonpath and --pants-config-files files that are in {buildroot} dir with those invalidation_globs provided by users. """ - invalidation_globs = set() - globs = set( - sys.path - + bootstrap_options.pythonpath - + bootstrap_options.pants_config_files - + bootstrap_options.pantsd_invalidation_globs - ) + invalidation_globs = OrderedSet() + globs = [ + *sys.path, + *bootstrap_options.pythonpath, + *bootstrap_options.pants_config_files, + "!*.pyc", + "!__pycache__/", + *bootstrap_options.pantsd_invalidation_globs, + ] for glob in globs: if glob.startswith("!"): @@ -155,7 +158,7 @@ def compute_pantsd_invalidation_globs(buildroot, bootstrap_options): f"Changes to {glob}, outside of the buildroot, will not be invalidated." ) - return list(sorted(invalidation_globs)) + return list(invalidation_globs) @classmethod def create(cls, options_bootstrapper, build_configuration, init_subsystems=True): diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index ea9e4a47033..d47ebff082b 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -486,15 +486,16 @@ def register_bootstrap_options(cls, register): help="Write logs to files under this directory.", ) - # This facilitates bootstrap-time configuration of pantsd usage such that we can - # determine whether or not to use the Pailgun client to invoke a given pants run - # without resorting to heavier options parsing. register( "--enable-pantsd", advanced=True, type=bool, default=False, - help="Enables use of the pants daemon (and implicitly, the v2 engine). (Beta)", + help=( + "Enables use of the pants daemon (pantsd). pantsd can significantly improve " + "runtime performance by lowering per-run startup cost, and by caching filesystem " + "operations and @rule execution." + ), ) # Whether or not to make necessary arrangements to have concurrent runs in pants. diff --git a/src/python/pants/testutil/pants_run_integration_test.py b/src/python/pants/testutil/pants_run_integration_test.py index 178e3179c42..7622de84875 100644 --- a/src/python/pants/testutil/pants_run_integration_test.py +++ b/src/python/pants/testutil/pants_run_integration_test.py @@ -8,11 +8,12 @@ import subprocess import sys import unittest -from contextlib import contextmanager +from contextlib import contextmanager, suppress from dataclasses import dataclass from operator import eq, ne +from pathlib import Path from threading import Lock -from typing import Any, Callable, List, Optional, Union +from typing import Any, Callable, Iterator, List, Optional, Union from colors import strip_color @@ -557,7 +558,34 @@ def file_renamed(self, prefix, test_name, real_name): os.rename(real_path, test_path) @contextmanager - def temporary_file_content(self, path, content, binary_mode=True): + def temporary_directory_literal(self, path: Union[str, Path],) -> Iterator[None]: + """Temporarily create the given literal directory under the buildroot. + + The path being created must not already exist. Any parent directories will also be created + temporarily. + """ + path = os.path.realpath(path) + assert path.startswith( + os.path.realpath(get_buildroot()) + ), "cannot write paths outside of the buildroot!" + assert not os.path.exists(path), "refusing to overwrite an existing path!" + + parent = os.path.dirname(path) + parent_ctx = ( + suppress() if os.path.isdir(parent) else self.temporary_directory_literal(parent) + ) + + with parent_ctx: + try: + os.mkdir(path) + yield + finally: + os.rmdir(path) + + @contextmanager + def temporary_file_content( + self, path: Union[str, Path], content, binary_mode=True + ) -> Iterator[None]: """Temporarily write content to a file for the purpose of an integration test.""" path = os.path.realpath(path) assert path.startswith( @@ -565,34 +593,51 @@ def temporary_file_content(self, path, content, binary_mode=True): ), "cannot write paths outside of the buildroot!" assert not os.path.exists(path), "refusing to overwrite an existing path!" mode = "wb" if binary_mode else "w" - with open(path, mode) as fh: - fh.write(content) - try: - yield - finally: - os.unlink(path) + + parent = os.path.dirname(path) + parent_ctx = ( + suppress() if os.path.isdir(parent) else self.temporary_directory_literal(parent) + ) + with parent_ctx: + try: + with open(path, mode) as fh: + fh.write(content) + yield + finally: + os.unlink(path) @contextmanager - def with_overwritten_file_content(self, file_path, temporary_content=None): + def with_overwritten_file_content( + self, + file_path: Union[str, Path], + temporary_content: Optional[Union[bytes, str, Callable[[bytes], bytes]]] = None, + ) -> Iterator[None]: """A helper that resets a file after the method runs. It will read a file, save the content, maybe write temporary_content to it, yield, then write the original content to the file. :param file_path: Absolute path to the file to be reset after the method runs. - :param temporary_content: Optional content to write into the file. + :param temporary_content: Content to write to the file, or a function from current content + to new temporary content. """ - with open(file_path, "r") as f: + with open(file_path, "rb") as f: file_original_content = f.read() try: if temporary_content is not None: - with open(file_path, "w") as f: - f.write(temporary_content) + if callable(temporary_content): + content = temporary_content(file_original_content) + elif isinstance(temporary_content, bytes): + content = temporary_content + else: + content = temporary_content.encode() + with open(file_path, "wb") as f: + f.write(content) yield finally: - with open(file_path, "w") as f: + with open(file_path, "wb") as f: f.write(file_original_content) @contextmanager diff --git a/tests/python/pants_test/backend/python/tasks/native/test_ctypes_integration.py b/tests/python/pants_test/backend/python/tasks/native/test_ctypes_integration.py index 53b4f1e41f1..3222f1f9388 100644 --- a/tests/python/pants_test/backend/python/tasks/native/test_ctypes_integration.py +++ b/tests/python/pants_test/backend/python/tasks/native/test_ctypes_integration.py @@ -15,7 +15,7 @@ from pants.testutil.pants_run_integration_test import PantsRunIntegrationTest from pants.util.collections import assert_single_element from pants.util.contextutil import temporary_dir -from pants.util.dirutil import is_executable, read_file, safe_file_dump +from pants.util.dirutil import is_executable from pants.util.enums import match from pants_test.backend.python.tasks.util.wheel import name_and_platform @@ -129,22 +129,13 @@ def test_ctypes_binary_creation(self, toolchain_variant): @_toolchain_variants def test_ctypes_native_language_interop(self, toolchain_variant): - # TODO: consider making this mock_buildroot/run_pants_with_workdir into a - # PantsRunIntegrationTest method! - with self.mock_buildroot( - dirs_to_copy=[self._binary_interop_target_dir] - ) as buildroot, buildroot.pushd(): - - # Replace strict_deps=False with nothing so we can override it (because target values for this - # option take precedence over subsystem options). - orig_wrapped_math_build = read_file(self._wrapped_math_build_file) - without_strict_deps_wrapped_math_build = re.sub( - "strict_deps=False,", "", orig_wrapped_math_build - ) - safe_file_dump(self._wrapped_math_build_file, without_strict_deps_wrapped_math_build) - + # Replace strict_deps=False with nothing so we can override it (because target values for this + # option take precedence over subsystem options). + with self.with_overwritten_file_content( + self._wrapped_math_build_file, lambda c: re.sub(b"strict_deps=False,", b"", c) + ): # This should fail because it does not turn on strict_deps for a target which requires it. - pants_binary_strict_deps_failure = self.run_pants_with_workdir( + pants_binary_strict_deps_failure = self.run_pants( command=["binary", self._binary_target_with_interop], # Explicitly set to True (although this is the default). config={ @@ -152,7 +143,6 @@ def test_ctypes_native_language_interop(self, toolchain_variant): # TODO(#6848): don't make it possible to forget to add the toolchain_variant option! "native-build-settings": {"strict_deps": True}, }, - workdir=os.path.join(buildroot.new_buildroot, ".pants.d"), ) self.assert_failure(pants_binary_strict_deps_failure) self.assertIn( diff --git a/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions_integration.py b/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions_integration.py index 6db81d14ae5..d91a4ec70aa 100644 --- a/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions_integration.py +++ b/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions_integration.py @@ -64,29 +64,15 @@ def test_pydist_invalidation(self): """Test that the current version of a python_dist() is resolved after modifying its sources.""" hello_run = f"{self.hello_install_requires_dir}:main_with_no_conflict" + run_target = lambda: self.run_pants(command=["--quiet", "run", hello_run]) - with self.mock_buildroot( - dirs_to_copy=[self.hello_install_requires_dir] - ) as buildroot, buildroot.pushd(): - run_target = lambda: self.run_pants_with_workdir( - command=["--quiet", "run", hello_run], - workdir=os.path.join(buildroot.new_buildroot, ".pants.d"), - build_root=buildroot.new_buildroot, - ) - - unmodified_pants_run = run_target() - self.assert_success(unmodified_pants_run) - self._assert_nation_and_greeting(unmodified_pants_run.stdout_data) - - # Modify one of the source files for this target so that the output is different. - py_source_file = os.path.join(self.hello_install_requires_dir, "hello_package/hello.py") - with open(py_source_file, "r") as f: - orig_contents = f.read() - # Replace hello! with hello? - modified_contents = re.sub("!", "?", orig_contents) - with open(py_source_file, "w") as f: - f.write(modified_contents) + unmodified_pants_run = run_target() + self.assert_success(unmodified_pants_run) + self._assert_nation_and_greeting(unmodified_pants_run.stdout_data) + # Modify one of the source files for this target so that the output is different. + py_source_file = os.path.join(self.hello_install_requires_dir, "hello_package/hello.py") + with self.with_overwritten_file_content(py_source_file, lambda c: re.sub(b"!", b"?", c)): modified_pants_run = run_target() self.assert_success(modified_pants_run) self._assert_nation_and_greeting(modified_pants_run.stdout_data, punctuation="?") diff --git a/tests/python/pants_test/backend/python/tasks/test_python_binary_integration.py b/tests/python/pants_test/backend/python/tasks/test_python_binary_integration.py index 1f92623086a..949f5942d8e 100644 --- a/tests/python/pants_test/backend/python/tasks/test_python_binary_integration.py +++ b/tests/python/pants_test/backend/python/tasks/test_python_binary_integration.py @@ -1,7 +1,6 @@ # Copyright 2016 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import functools import glob import os import subprocess @@ -53,31 +52,31 @@ def test_zipsafe_caching(self): test_pex = "dist/cache_fields.pex" zipsafe_target_tmpl = "python_binary(sources=['main.py'], zip_safe={})" - with self.caching_config() as config, self.mock_buildroot() as buildroot, buildroot.pushd(): - build = functools.partial( - self.run_pants_with_workdir, - command=["binary", test_project], - workdir=os.path.join(buildroot.new_buildroot, ".pants.d"), - config=config, - build_root=buildroot.new_buildroot, + with self.caching_config() as config, self.temporary_workdir() as workdir, self.temporary_file_content( + test_src, b"" + ): + build = lambda: self.run_pants_with_workdir( + command=["binary", test_project], config=config, workdir=workdir ) - buildroot.write_file(test_src, "") - # Create a pex from a simple python_binary target and assert it has zip_safe=True (default). - buildroot.write_file(test_build, "python_binary(sources=['main.py'])") - self.assert_success(build()) - self.assert_pex_attribute(test_pex, "zip_safe", True) + with self.temporary_file_content(test_build, b"python_binary(sources=['main.py'])"): + self.assert_success(build()) + self.assert_pex_attribute(test_pex, "zip_safe", True) # Simulate a user edit by adding zip_safe=False to the target and check the resulting pex. - buildroot.write_file(test_build, zipsafe_target_tmpl.format("False")) - self.assert_success(build()) - self.assert_pex_attribute(test_pex, "zip_safe", False) + with self.temporary_file_content( + test_build, zipsafe_target_tmpl.format("False").encode() + ): + self.assert_success(build()) + self.assert_pex_attribute(test_pex, "zip_safe", False) # Simulate a user edit by adding zip_safe=True to the target and check the resulting pex. - buildroot.write_file(test_build, zipsafe_target_tmpl.format("True")) - self.assert_success(build()) - self.assert_pex_attribute(test_pex, "zip_safe", True) + with self.temporary_file_content( + test_build, zipsafe_target_tmpl.format("True").encode() + ): + self.assert_success(build()) + self.assert_pex_attribute(test_pex, "zip_safe", True) def test_platform_defaults_to_config(self): self.platforms_test_impl( @@ -133,7 +132,7 @@ def assertInAny(substring, collection): def assertNotInAny(substring, collection): self.assertTrue( all(substring not in d for d in collection), - f'Expected an entry matching "{substring}" in {collection}', + f'Expected no entries matching "{substring}" in {collection}', ) test_project = "testprojects/src/python/cache_fields" @@ -141,15 +140,11 @@ def assertNotInAny(substring, collection): test_src = os.path.join(test_project, "main.py") test_pex = "dist/cache_fields.pex" - with self.caching_config() as config, self.mock_buildroot() as buildroot, buildroot.pushd(): + with self.caching_config() as config, self.temporary_file_content(test_src, b""): config["python-setup"] = {"platforms": []} - buildroot.write_file(test_src, "") - - buildroot.write_file( - test_build, - dedent( - """ + build_content = dedent( + """ python_binary( sources=['main.py'], dependencies=[':numpy'], @@ -163,26 +158,22 @@ def assertNotInAny(substring, collection): ) """.format( - target_platforms="platforms = [{}],".format( - ", ".join(["'{}'".format(p) for p in target_platforms]) - ) - if target_platforms is not None - else "", + target_platforms="platforms = [{}],".format( + ", ".join(["'{}'".format(p) for p in target_platforms]) ) - ), - ) - # When only the linux platform is requested, - # only linux wheels should end up in the pex. - if config_platforms is not None: - config["python-setup"]["platforms"] = config_platforms - result = self.run_pants_with_workdir( - command=["binary", test_project], - workdir=os.path.join(buildroot.new_buildroot, ".pants.d"), - config=config, - build_root=buildroot.new_buildroot, - tee_output=True, + if target_platforms is not None + else "", + ) ) - self.assert_success(result) + with self.temporary_file_content(test_build, build_content.encode()): + # When only the linux platform is requested, + # only linux wheels should end up in the pex. + if config_platforms is not None: + config["python-setup"]["platforms"] = config_platforms + result = self.run_pants( + command=["binary", test_project], config=config, tee_output=True, + ) + self.assert_success(result) with open_zip(test_pex) as z: deps = p537_deps(z.namelist())