diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index f50eb4ea344..1e759894f16 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -4,6 +4,7 @@ beautifulsoup4>=4.6.0,<4.7 cffi==1.11.5 contextlib2==0.5.5 coverage>=4.5,<4.6 +dataclasses==0.6 docutils==0.14 fasteners==0.14.1 Markdown==2.1.1 diff --git a/src/python/pants/backend/python/subsystems/pex_build_util.py b/src/python/pants/backend/python/subsystems/pex_build_util.py index c2d41b8fea6..f152531a59f 100644 --- a/src/python/pants/backend/python/subsystems/pex_build_util.py +++ b/src/python/pants/backend/python/subsystems/pex_build_util.py @@ -4,6 +4,8 @@ import logging import os from collections import defaultdict +from pathlib import Path +from typing import Callable, Sequence, Set from pex.fetcher import Fetcher from pex.pex_builder import PEXBuilder @@ -22,39 +24,40 @@ from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError from pants.build_graph.files import Files +from pants.build_graph.target import Target from pants.subsystem.subsystem import Subsystem from pants.util.collections import assert_single_element from pants.util.contextutil import temporary_file -def is_python_target(tgt): +def is_python_target(tgt: Target) -> bool: # We'd like to take all PythonTarget subclasses, but currently PythonThriftLibrary and # PythonAntlrLibrary extend PythonTarget, and until we fix that (which we can't do until # we remove the old python pipeline entirely) we want to ignore those target types here. return isinstance(tgt, (PythonLibrary, PythonTests, PythonBinary)) -def has_python_sources(tgt): +def has_python_sources(tgt: Target) -> bool: return is_python_target(tgt) and tgt.has_sources() -def is_local_python_dist(tgt): +def is_local_python_dist(tgt: Target) -> bool: return isinstance(tgt, PythonDistribution) -def has_resources(tgt): +def has_resources(tgt: Target) -> bool: return isinstance(tgt, Files) and tgt.has_sources() -def has_python_requirements(tgt): +def has_python_requirements(tgt: Target) -> bool: return isinstance(tgt, PythonRequirementLibrary) -def always_uses_default_python_platform(tgt): +def always_uses_default_python_platform(tgt: Target) -> bool: return isinstance(tgt, PythonTests) -def may_have_explicit_python_platform(tgt): +def may_have_explicit_python_platform(tgt: Target) -> bool: return isinstance(tgt, PythonBinary) @@ -76,7 +79,7 @@ def targets_by_platform(targets, python_setup): return dict(explicit_platform_settings) -def identify_missing_init_files(sources): +def identify_missing_init_files(sources: Sequence[str]) -> Set[str]: """Return the list of paths that would need to be added to ensure that every package has an __init__.py. """ packages = set() @@ -92,39 +95,29 @@ def identify_missing_init_files(sources): return {os.path.join(package, '__init__.py') for package in packages} - set(sources) -def _create_source_dumper(builder, tgt): - if type(tgt) == Files: - # Loose `Files` as opposed to `Resources` or `PythonTarget`s have no (implied) package structure - # and so we chroot them relative to the build root so that they can be accessed via the normal - # python filesystem APIs just as they would be accessed outside the chrooted environment. - # NB: This requires we mark the pex as not zip safe so these `Files` can still be accessed in - # the context of a built pex distribution. - chroot_path = lambda relpath: relpath - builder.info.zip_safe = False - else: - chroot_path = lambda relpath: os.path.relpath(relpath, tgt.target_base) - - dump = builder.add_resource if has_resources(tgt) else builder.add_source +def _create_source_dumper(builder: PEXBuilder, tgt: Target) -> Callable[[str], None]: buildroot = get_buildroot() - return lambda relpath: dump(os.path.join(buildroot, relpath), chroot_path(relpath)) + def get_chroot_path(relpath: str) -> str: + if type(tgt) == Files: + # Loose `Files`, as opposed to `Resources` or `PythonTarget`s, have no (implied) package + # structure and so we chroot them relative to the build root so that they can be accessed + # via the normal Python filesystem APIs just as they would be accessed outside the + # chrooted environment. NB: This requires we mark the pex as not zip safe so + # these `Files` can still be accessed in the context of a built pex distribution. + builder.info.zip_safe = False + return relpath + return str(Path(relpath).relative_to(tgt.target_base)) -def dump_sources(builder, tgt, log): - dump_source = _create_source_dumper(builder, tgt) - log.debug(' Dumping sources: {}'.format(tgt)) - for relpath in tgt.sources_relative_to_buildroot(): - try: - dump_source(relpath) - except OSError: - log.error('Failed to copy {} for target {}'.format(relpath, tgt.address.spec)) - raise + def dump_source(relpath: str) -> None: + source_path = str(Path(buildroot) / relpath) + dest_path = get_chroot_path(relpath) + if has_resources(tgt): + builder.add_resource(filename=source_path, env_filename=dest_path) + else: + builder.add_source(filename=source_path, env_filename=dest_path) - if (getattr(tgt, '_resource_target_specs', None) or - getattr(tgt, '_synthetic_resources_target', None)): - # No one should be on old-style resources any more. And if they are, - # switching to the new python pipeline will be a great opportunity to fix that. - raise TaskError('Old-style resources not supported for target {}. ' - 'Depend on resources() targets instead.'.format(tgt.address.spec)) + return dump_source class PexBuilderWrapper: @@ -150,7 +143,7 @@ def subsystem_dependencies(cls): @classmethod def create(cls, builder, log=None): options = cls.global_instance().get_options() - setuptools_requirement = 'setuptools=={}'.format(options.setuptools_version) + setuptools_requirement = f'setuptools=={options.setuptools_version}' log = log or logging.getLogger(__name__) @@ -216,15 +209,15 @@ def extract_single_dist_for_current_platform(self, reqs, dist_key): )) except (StopIteration, ValueError) as e: raise self.SingleDistExtractionError( - "Exactly one dist was expected to match name {} in requirements {}: {}" - .format(dist_key, reqs, e)) + f"Exactly one dist was expected to match name {dist_key} in requirements {reqs}: {e!r}" + ) return matched_dist def _resolve_distributions_by_platform(self, reqs, platforms): deduped_reqs = OrderedSet(reqs) find_links = OrderedSet() for req in deduped_reqs: - self._log.debug(' Dumping requirement: {}'.format(req)) + self._log.debug(f' Dumping requirement: {req}') self._builder.add_requirement(str(req.requirement)) if req.repository: find_links.add(req.repository) @@ -249,7 +242,7 @@ def add_resolved_requirements(self, reqs, platforms=None): for platform, dists in distributions.items(): for dist in dists: if dist.location not in locations: - self._log.debug(' Dumping distribution: .../{}'.format(os.path.basename(dist.location))) + self._log.debug(f' Dumping distribution: .../{os.path.basename(dist.location)}') self.add_distribution(dist) locations.add(dist.location) @@ -291,24 +284,26 @@ def _resolve_multi(self, interpreter, requirements, platforms, find_links): return distributions - def add_sources_from(self, tgt): + def add_sources_from(self, tgt: Target) -> None: dump_source = _create_source_dumper(self._builder, tgt) - self._log.debug(' Dumping sources: {}'.format(tgt)) + self._log.debug(f' Dumping sources: {tgt}') for relpath in tgt.sources_relative_to_buildroot(): try: dump_source(relpath) except OSError: - self._log.error('Failed to copy {} for target {}'.format(relpath, tgt.address.spec)) + self._log.error(f'Failed to copy {relpath} for target {tgt.address.spec}') raise if (getattr(tgt, '_resource_target_specs', None) or getattr(tgt, '_synthetic_resources_target', None)): # No one should be on old-style resources any more. And if they are, # switching to the new python pipeline will be a great opportunity to fix that. - raise TaskError('Old-style resources not supported for target {}. ' - 'Depend on resources() targets instead.'.format(tgt.address.spec)) + raise TaskError( + f'Old-style resources not supported for target {tgt.address.spec}. Depend on resources() ' + 'targets instead.' + ) - def _prepare_inits(self): + def _prepare_inits(self) -> Set[str]: chroot = self._builder.chroot() sources = chroot.get('source') | chroot.get('resource') missing_init_files = identify_missing_init_files(sources) @@ -317,20 +312,21 @@ def _prepare_inits(self): ns_package.write(b'__import__("pkg_resources").declare_namespace(__name__)') ns_package.flush() for missing_init_file in missing_init_files: - self._builder.add_source(ns_package.name, missing_init_file) + self._builder.add_source(filename=ns_package.name, env_filename=missing_init_file) return missing_init_files def set_emit_warnings(self, emit_warnings): self._builder.info.emit_warnings = emit_warnings - def freeze(self): - if not self._frozen: - if self._prepare_inits(): - dist = self._distributions.get('setuptools') - if not dist: - self.add_resolved_requirements([self._setuptools_requirement]) - self._builder.freeze(bytecode_compile=False) - self._frozen = True + def freeze(self) -> None: + if self._frozen: + return + if self._prepare_inits(): + dist = self._distributions.get('setuptools') + if not dist: + self.add_resolved_requirements([self._setuptools_requirement]) + self._builder.freeze(bytecode_compile=False) + self._frozen = True def set_entry_point(self, entry_point): self._builder.set_entry_point(entry_point) diff --git a/tests/python/pants_test/BUILD b/tests/python/pants_test/BUILD index cd541553ebe..70ca1bc968d 100644 --- a/tests/python/pants_test/BUILD +++ b/tests/python/pants_test/BUILD @@ -29,16 +29,20 @@ python_library( ], dependencies = [ '3rdparty/python:ansicolors', + '3rdparty/python:dataclasses', 'build-support/regexes', 'src/python/pants/base:build_environment', 'src/python/pants/base:build_file', + 'src/python/pants/base:exiter', 'src/python/pants/fs', 'src/python/pants/subsystem', 'src/python/pants/util:contextutil', 'src/python/pants/util:dirutil', + 'src/python/pants/util:objects', + 'src/python/pants/util:osutil', 'src/python/pants/util:process_handler', + 'src/python/pants/util:strutil', 'tests/python/pants_test/testutils:file_test_util', - 'tests/python/pants_test/testutils:pexrc_util', ] ) diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile_integration.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile_integration.py index 4d0fbd1c268..efa43978a3c 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile_integration.py @@ -67,12 +67,12 @@ def basic_binary(self, config): pants_run.workdir, 'compile/rsc/current/testprojects.src.scala.org.pantsbuild.testproject.mutual.mutual/current/zinc', 'classes/org/pantsbuild/testproject/mutual/A.class') - self.assertIsFile(zinc_compiled_classfile) + self.assert_is_file(zinc_compiled_classfile) rsc_header_jar = os.path.join( pants_run.workdir, 'compile/rsc/current/testprojects.src.scala.org.pantsbuild.testproject.mutual.mutual/current/rsc', 'm.jar') - self.assertIsFile(rsc_header_jar) + self.assert_is_file(rsc_header_jar) @_for_all_supported_execution_environments def executing_multi_target_binary(self, config): diff --git a/tests/python/pants_test/pants_run_integration_test.py b/tests/python/pants_test/pants_run_integration_test.py index 69afbad8825..5231a1d214d 100644 --- a/tests/python/pants_test/pants_run_integration_test.py +++ b/tests/python/pants_test/pants_run_integration_test.py @@ -9,8 +9,10 @@ import subprocess import unittest from contextlib import contextmanager +from dataclasses import dataclass from operator import eq, ne from threading import Lock +from typing import List, Optional, Union from colors import strip_color @@ -21,33 +23,36 @@ from pants.subsystem.subsystem import Subsystem from pants.util.contextutil import environment_as, pushd, temporary_dir from pants.util.dirutil import fast_relpath, safe_mkdir, safe_mkdir_for, safe_open -from pants.util.objects import Exactly, datatype, string_type +from pants.util.objects import datatype from pants.util.osutil import Pid from pants.util.process_handler import SubprocessProcessHandler from pants.util.strutil import ensure_binary from pants_test.testutils.file_test_util import check_symlinks, contains_exact_files -class PantsResult(datatype([ - # NB: May be either a string list or string (in the case of `shell=True`). - 'command', - ('returncode', int), - ('stdout_data', string_type), - ('stderr_data', string_type), - ('workdir', string_type), - ('pid', Exactly(Pid)), -])): - pass +# NB: If `shell=True`, it's a single `str`. +Command = Union[str, List[str]] -class PantsJoinHandle(datatype([ - # NB: May be either a string list or string (in the case of `shell=True`). - 'command', - 'process', - ('workdir', string_type), -])): +@dataclass(frozen=True) +class PantsResult: + command: Command + returncode: int + stdout_data: str + stderr_data: str + workdir: str + pid: Pid - def join(self, stdin_data=None, tee_output=False): + +@dataclass(frozen=True) +class PantsJoinHandle: + command: Command + process: subprocess.Popen + workdir: str + + def join( + self, stdin_data: Optional[Union[bytes, str]] = None, tee_output: bool = False + ) -> PantsResult: """Wait for the pants process to complete, and return a PantsResult for it.""" communicate_fn = self.process.communicate @@ -61,12 +66,13 @@ def join(self, stdin_data=None, tee_output=False): render_logs(self.workdir) return PantsResult( - self.command, - self.process.returncode, - stdout_data.decode(), - stderr_data.decode(), - self.workdir, - self.process.pid) + command=self.command, + returncode=self.process.returncode, + stdout_data=stdout_data.decode(), + stderr_data=stderr_data.decode(), + workdir=self.workdir, + pid=self.process.pid + ) def ensure_cached(expected_num_artifacts=None): @@ -80,7 +86,7 @@ def ensure_cached(expected_num_artifacts=None): def decorator(test_fn): def wrapper(self, *args, **kwargs): with temporary_dir() as artifact_cache: - cache_args = '--cache-write-to=["{}"]'.format(artifact_cache) + cache_args = f'--cache-write-to=["{artifact_cache}"]' test_fn(self, *args + (cache_args,), **kwargs) @@ -125,7 +131,7 @@ def wrapper(self, *args, **kwargs): if enable_daemon: self.assert_success(self.run_pants(['kill-pantsd'])) except Exception: - print('Test failed with enable-pantsd={}:'.format(enable_daemon)) + print(f'Test failed with enable-pantsd={enable_daemon}:') if enable_daemon: # If we are already raising, do not attempt to confirm that `kill-pantsd` succeeds. self.run_pants(['kill-pantsd']) @@ -144,16 +150,16 @@ def render_logs(workdir): ) for filename in filenames: rel_filename = fast_relpath(filename, workdir) - print('{} +++ '.format(rel_filename)) + print(f'{rel_filename} +++ ') for line in _read_log(filename): - print('{} >>> {}'.format(rel_filename, line)) - print('{} --- '.format(rel_filename)) + print(f'{rel_filename} >>> {line}') + print(f'{rel_filename} --- ') def read_pantsd_log(workdir): """Yields all lines from the pantsd log under the given workdir.""" # Surface the pantsd log for easy viewing via pytest's `-s` (don't capture stdio) option. - for line in _read_log('{}/pantsd/pantsd.log'.format(workdir)): + for line in _read_log(f'{workdir}/pantsd/pantsd.log'): yield line @@ -289,9 +295,9 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None, **kwargs): args = [ '--no-pantsrc', - '--pants-workdir={}'.format(workdir), + f'--pants-workdir={workdir}', '--kill-nailguns', - '--print-exception-stacktrace={}'.format(print_exception_stacktrace), + f'--print-exception-stacktrace={print_exception_stacktrace}', ] if self.hermetic(): @@ -352,15 +358,15 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None, # Don't overwrite the profile of this process in the called process. # Instead, write the profile into a sibling file. if env.get('PANTS_PROFILE'): - prof = '{}.{}'.format(env['PANTS_PROFILE'], self._get_profile_disambiguator()) + prof = f"{env['PANTS_PROFILE']}.{self._get_profile_disambiguator()}" env['PANTS_PROFILE'] = prof # Make a note the subprocess command, so the user can correctly interpret the profile files. - with open('{}.cmd'.format(prof), 'w') as fp: + with open(f'{prof}.cmd', 'w') as fp: fp.write(' '.join(pants_command)) return PantsJoinHandle( - pants_command, - subprocess.Popen( + command=pants_command, + process=subprocess.Popen( pants_command, env=env, stdin=subprocess.PIPE, @@ -368,24 +374,26 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None, stderr=subprocess.PIPE, **kwargs ), - workdir + workdir=workdir ) - def run_pants_with_workdir(self, command, workdir, config=None, stdin_data=None, tee_output=False, **kwargs): + def run_pants_with_workdir( + self, command, workdir, config=None, stdin_data=None, tee_output=False, **kwargs + ) -> PantsResult: if config: kwargs["config"] = config handle = self.run_pants_with_workdir_without_waiting(command, workdir, **kwargs) return handle.join(stdin_data=stdin_data, tee_output=tee_output) - def run_pants(self, command, config=None, stdin_data=None, extra_env=None, cleanup_workdir=True, - **kwargs): + def run_pants( + self, command, config=None, stdin_data=None, extra_env=None, cleanup_workdir=True, **kwargs + ) -> PantsResult: """Runs pants in a subprocess. :param list command: A list of command line arguments coming after `./pants`. :param config: Optional data for a generated ini file. A map of -> map of key -> value. If order in the ini file matters, this should be an OrderedDict. :param kwargs: Extra keyword args to pass to `subprocess.Popen`. - :returns a PantsResult instance. """ with self.temporary_workdir() as workdir: return self.run_pants_with_workdir( @@ -444,8 +452,7 @@ def bundle_and_run(self, target, bundle_name, bundle_jar_name=None, bundle_optio with self.pants_results(bundle_options) as pants_run: self.assert_success(pants_run) - self.assertTrue(check_symlinks('dist/{bundle_name}-bundle/libs'.format(bundle_name=bundle_name), - library_jars_are_symlinks)) + self.assertTrue(check_symlinks(f'dist/{bundle_name}-bundle/libs', library_jars_are_symlinks)) # TODO(John Sirois): We need a zip here to suck in external library classpath elements # pointed to by symlinks in the run_pants ephemeral tmpdir. Switch run_pants to be a # contextmanager that yields its results while the tmpdir workdir is still active and change @@ -456,51 +463,49 @@ def bundle_and_run(self, target, bundle_name, bundle_jar_name=None, bundle_optio self.assertTrue(contains_exact_files(workdir, expected_bundle_content)) if expected_bundle_jar_content: with temporary_dir() as check_bundle_jar_dir: - bundle_jar = os.path.join(workdir, '{bundle_jar_name}.jar' - .format(bundle_jar_name=bundle_jar_name)) + bundle_jar = os.path.join(workdir, f'{bundle_jar_name}.jar') ZIP.extract(bundle_jar, check_bundle_jar_dir) self.assertTrue(contains_exact_files(check_bundle_jar_dir, expected_bundle_jar_content)) optional_args = [] if args: optional_args = args - java_run = subprocess.Popen(['java', - '-jar', - '{bundle_jar_name}.jar'.format(bundle_jar_name=bundle_jar_name)] - + optional_args, - stdout=subprocess.PIPE, - cwd=workdir) + java_run = subprocess.Popen( + ['java', '-jar', f'{bundle_jar_name}.jar'] + optional_args, + stdout=subprocess.PIPE, + cwd=workdir + ) stdout, _ = java_run.communicate() java_returncode = java_run.returncode self.assertEqual(java_returncode, 0) return stdout.decode() - def assert_success(self, pants_run, msg=None): + def assert_success(self, pants_run: PantsResult, msg=None): self.assert_result(pants_run, PANTS_SUCCEEDED_EXIT_CODE, expected=True, msg=msg) - def assert_failure(self, pants_run, msg=None): + def assert_failure(self, pants_run: PantsResult, msg=None): self.assert_result(pants_run, PANTS_SUCCEEDED_EXIT_CODE, expected=False, msg=msg) - def assert_result(self, pants_run, value, expected=True, msg=None): + def assert_result(self, pants_run: PantsResult, value, expected=True, msg=None): check, assertion = (eq, self.assertEqual) if expected else (ne, self.assertNotEqual) if check(pants_run.returncode, value): return details = [msg] if msg else [] details.append(' '.join(pants_run.command)) - details.append('returncode: {returncode}'.format(returncode=pants_run.returncode)) + details.append(f'returncode: {pants_run.returncode}') def indent(content): return '\n\t'.join(content.splitlines()) - details.append('stdout:\n\t{stdout}'.format(stdout=indent(pants_run.stdout_data))) - details.append('stderr:\n\t{stderr}'.format(stderr=indent(pants_run.stderr_data))) + details.append(f'stdout:\n\t{indent(pants_run.stdout_data)}') + details.append(f'stderr:\n\t{indent(pants_run.stderr_data)}') error_msg = '\n'.join(details) assertion(value, pants_run.returncode, error_msg) - def assert_run_contains_log(self, msg, level, module, pants_run): + def assert_run_contains_log(self, msg, level, module, pants_run: PantsResult): """Asserts that the passed run's stderr contained the log message.""" self.assert_contains_log(msg, level, module, pants_run.stderr_data, pants_run.pid) @@ -511,14 +516,17 @@ def assert_contains_log(self, msg, level, module, log, pid=None): If pid is specified, performs an exact match including the pid of the pants process. Otherwise performs a regex match asserting that some pid is present. """ - prefix = "[{}] {}:pid=".format(level, module) - suffix = ": {}".format(msg) + prefix = f"[{level}] {module}:pid=" + suffix = f": {msg}" if pid is None: - self.assertRegex(log, re.escape(prefix) + "\\d+" + re.escape(suffix)) + self.assertRegex(log, re.escape(prefix) + r"\d+" + re.escape(suffix)) else: - self.assertIn("{}{}{}".format(prefix, pid, suffix), log) + self.assertIn(f"{prefix}{pid}{suffix}", log) + + def assert_is_file(self, file_path): + self.assertTrue(os.path.isfile(file_path), f'file path {file_path} does not exist!') - def normalize(self, s): + def normalize(self, s: str) -> str: """Removes escape sequences (e.g. colored output) and all whitespace from string s.""" return ''.join(strip_color(s).split()) @@ -613,11 +621,10 @@ def dir_context(): yield Manager(write_file, dir_context, tmp_dir) - def do_command(self, *args, **kwargs): + def do_command(self, *args, **kwargs) -> PantsResult: """Wrapper around run_pants method. :param args: command line arguments used to run pants - :return: a PantsResult object """ cmd = list(args) success = kwargs.pop('success', True) @@ -638,6 +645,3 @@ def do_command_yielding_workdir(self, *args, **kwargs): else: self.assert_failure(pants_run) yield pants_run - - def assertIsFile(self, file_path): - self.assertTrue(os.path.isfile(file_path), 'file path {} does not exist!'.format(file_path))