diff --git a/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py b/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py index a604ceee26c..7f7841c4455 100644 --- a/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py +++ b/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py @@ -301,4 +301,4 @@ def _resolved_export_info(self): @property def _copy_target_attributes(self): - return ['provides', 'strict_deps', 'fatal_warnings'] + return ['provides', 'strict_deps'] diff --git a/contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py b/contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py index 6a641a413ee..fe4b8f20b14 100644 --- a/contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py +++ b/contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py @@ -98,7 +98,6 @@ def _test_create_build_str(self, language, compiler_args): language='{language}', compiler_args={compiler_args}, strict_deps=True, - fatal_warnings=False, ) '''.format(language=language, compiler_args=compiler_args_str)) @@ -134,7 +133,6 @@ def _test_help(self, language, library_type, compiler_args, sources): self.assertEqual(call_kwargs['provides'], None) self.assertEqual(call_kwargs['derived_from'], target) self.assertEqual(call_kwargs['strict_deps'], True) - self.assertEqual(call_kwargs['fatal_warnings'], False) sources = call_kwargs['sources'] self.assertEqual(sources.files, ()) diff --git a/src/python/pants/VERSION b/src/python/pants/VERSION index cc7e9a85c04..12790c22b44 100644 --- a/src/python/pants/VERSION +++ b/src/python/pants/VERSION @@ -1 +1 @@ -1.10.0rc0 +1.11.0.dev0 diff --git a/src/python/pants/backend/jvm/subsystems/zinc_language_mixin.py b/src/python/pants/backend/jvm/subsystems/zinc_language_mixin.py index 6a5d413e347..229b2c1ffb1 100644 --- a/src/python/pants/backend/jvm/subsystems/zinc_language_mixin.py +++ b/src/python/pants/backend/jvm/subsystems/zinc_language_mixin.py @@ -6,8 +6,6 @@ from builtins import object -from pants.base.deprecated import deprecated - class ZincLanguageMixin(object): """A mixin for subsystems for languages compiled with Zinc.""" @@ -21,12 +19,6 @@ def register_options(cls, register): register('--strict-deps', advanced=True, default=False, fingerprint=True, type=bool, help='The default for the "strict_deps" argument for targets of this language.') - register('--fatal-warnings', advanced=True, type=bool, - fingerprint=True, - removal_version='1.11.0.dev0', - removal_hint='Use --compiler-option-sets=fatal_warnings instead of fatal_warnings', - help='The default for the "fatal_warnings" argument for targets of this language.') - register('--compiler-option-sets', advanced=True, default=[], type=list, fingerprint=True, help='The default for the "compiler_option_sets" argument ' @@ -43,24 +35,13 @@ def strict_deps(self): """ return self.get_options().strict_deps - @property - @deprecated('1.11.0.dev0', 'Consume fatal_warnings from compiler_option_sets instead.') - def fatal_warnings(self): - """If true, make warnings fatal for targets that do not specify fatal_warnings. - :rtype: bool - """ - return self.get_options().fatal_warnings - @property def compiler_option_sets(self): """For every element in this list, enable the corresponding flags on compilation of targets. :rtype: list """ - option_sets = self.get_options().compiler_option_sets - if 'fatal_warnings' not in option_sets and self.get_options().fatal_warnings: - option_sets.append('fatal_warnings') - return option_sets + return self.get_options().compiler_option_sets @property def zinc_file_manager(self): diff --git a/src/python/pants/backend/jvm/targets/jvm_target.py b/src/python/pants/backend/jvm/targets/jvm_target.py index 9d687a93393..bf61b72f44b 100644 --- a/src/python/pants/backend/jvm/targets/jvm_target.py +++ b/src/python/pants/backend/jvm/targets/jvm_target.py @@ -10,7 +10,6 @@ from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform from pants.backend.jvm.targets.jar_library import JarLibrary from pants.backend.jvm.targets.jarable import Jarable -from pants.base.deprecated import deprecated_conditional from pants.base.payload import Payload from pants.base.payload_field import ExcludesField, PrimitiveField, PrimitivesSetField from pants.build_graph.resources import Resources @@ -39,7 +38,6 @@ def __init__(self, platform=None, strict_deps=None, exports=None, - fatal_warnings=None, compiler_option_sets=None, zinc_file_manager=None, # Some subclasses can have both .java and .scala sources @@ -83,8 +81,6 @@ def __init__(self, dependents have access to the closure of exports. An example will be that if A exports B, and B exports C, then any targets that depends on A will have access to both B and C. - :param bool fatal_warnings: Whether to turn warnings into errors for this target. If present, - takes priority over the language's fatal-warnings option. Deprecated. :param bool zinc_file_manager: Whether to use zinc provided file manager that allows transactional rollbacks, but in certain cases may conflict with user libraries. @@ -97,21 +93,6 @@ def __init__(self, self.address = address # Set in case a TargetDefinitionException is thrown early payload = payload or Payload() excludes = ExcludesField(self.assert_list(excludes, expected_type=Exclude, key_arg='excludes')) - deprecated_conditional( - lambda: fatal_warnings is not None, - removal_version='1.11.0dev0', - entity_description='fatal_warnings', - hint_message="fatal_warnings should be defined as part of the target compiler_option_sets" - ) - if fatal_warnings is not None: - compiler_option_sets = [] if compiler_option_sets is None else compiler_option_sets - if fatal_warnings: - compiler_option_sets.append('fatal_warnings') - else: - try: - compiler_option_sets.remove('fatal_warnings') - except ValueError: - pass payload.add_fields({ 'sources': self.create_sources_field(sources, address.spec_path, key_arg='sources'), 'provides': provides, @@ -146,18 +127,6 @@ def strict_deps(self): def export_specs(self): return self.payload.exports - @property - def fatal_warnings(self): - """If set, overrides the platform's default fatal_warnings setting. - - :return: See constructor. - :rtype: bool or None - """ - if self.payload.compiler_option_sets is not None: - return 'fatal_warnings' in self.payload.compiler_option_sets - else: - return False - @memoized_property def compiler_option_sets(self): """For every element in this list, enable the corresponding flags on compilation diff --git a/src/python/pants/build_graph/target.py b/src/python/pants/build_graph/target.py index 6006d125db2..b64a453d27b 100644 --- a/src/python/pants/build_graph/target.py +++ b/src/python/pants/build_graph/target.py @@ -26,7 +26,7 @@ from pants.build_graph.target_scopes import Scope from pants.fs.fs import safe_filename from pants.source.payload_fields import SourcesField -from pants.source.wrapped_globs import EagerFilesetWithSpec, Files, FilesetWithSpec, Globs +from pants.source.wrapped_globs import EagerFilesetWithSpec, FilesetWithSpec, Globs from pants.subsystem.subsystem import Subsystem from pants.util.memo import memoized_property @@ -864,38 +864,8 @@ def create_sources_field(self, sources, sources_rel_path, key_arg=None): :return: a payload field object representing the sources parameter :rtype: SourcesField """ - if sources is None: - # Make sure we don't apply the defaulting to uses of this method other than for - # creating a sources= field (e.g., we also use this for creating resources= fields). - # Note that the check for supports_default_sources() precedes the subsystem check. - # This is so that tests don't need to set up the subsystem when creating targets that - # legitimately do not require sources. - if (key_arg is None or key_arg == 'sources') and self.supports_default_sources(): - deprecated_conditional( - lambda: True, - '1.11.0.dev0', - 'Default sources should always be parsed through the engine not by create_sources_field. ' - 'This code should be unreachable, and this message should never be displayed. ' - 'If you see this message, please contact pants-dev. ' - 'Class which caused this message: {}'.format(self.__class__.__name__) - ) - sources = self.default_sources(sources_rel_path) - else: - sources = FilesetWithSpec.empty(sources_rel_path) - elif isinstance(sources, (set, list, tuple)): - if sources: - # Received a literal sources list: convert to a FilesetWithSpec via Files. - deprecated_conditional( - lambda: True, - '1.11.0.dev0', - ('Passing collections as the value of the sources argument to create_sources_field is ' - 'deprecated, and now takes a slow path. Instead, class {} should have its sources ' - 'argument populated by the engine, either by using the standard parsing pipeline, or by ' - 'requesting a SourcesField product from the v2 engine.').format(self.__class__.__name__) - ) - sources = Files.create_fileset_with_spec(sources_rel_path, *sources) - else: - sources = FilesetWithSpec.empty(sources_rel_path) + if not sources: + sources = FilesetWithSpec.empty(sources_rel_path) elif not isinstance(sources, FilesetWithSpec): key_arg_section = "'{}' to be ".format(key_arg) if key_arg else "" raise TargetDefinitionException(self, "Expected {}a glob, an address or a list, but was {}" @@ -903,7 +873,7 @@ def create_sources_field(self, sources, sources_rel_path, key_arg=None): elif not isinstance(sources, EagerFilesetWithSpec): deprecated_conditional( lambda: True, - '1.11.0.dev0', + '1.12.0.dev0', ('FilesetWithSpec sources values are deprecated except for EagerFilesetWithSpec values. ' 'Saw value of type {}').format(type(sources)) ) diff --git a/src/python/pants/notes/master.rst b/src/python/pants/notes/master.rst index da0097907ba..83cf710b9dd 100644 --- a/src/python/pants/notes/master.rst +++ b/src/python/pants/notes/master.rst @@ -4,6 +4,69 @@ Master Pre-Releases This document describes ``dev`` releases which occur weekly from master, and which do not undergo the vetting associated with ``stable`` releases. +1.11.0.dev0 (09/14/2018) +------------------------ + +API Changes +~~~~~~~~~~~ + +* Upgrade Node.js to 8.11.3 and Yarn to 1.6.0 (#6512) + `PR #6512 `_ + +New features +~~~~~~~~~~~~ + +* Add extra_jvm_options to jvm_binary targets (#6310) + `PR #6310 `_ + +* [compile.rsc] Add strategy for compiling with Rsc and Zinc (#6408) + `PR #6408 `_ + +* Add support for HTTP basic auth. (#6495) + `PR #6495 `_ + +* gRPC support for golang protobufs. (#6507) + `PR #6507 `_ + +Bugfixes +~~~~~~~~ + +* make fatal_warnings_enabled_args a tuple instead of just parens (#6497) + `PR #6497 `_ + +* pass through `compatibility` to synthetic python thrift targets (#6499) + `PR #6499 `_ + +* Apply workaround similer to #6409 to bootstrapper (#6498) + `PR #6498 `_ + +* Fix encoding of workunits under pantsd (#6505) + `PR #6505 `_ + +* refactor command line target spec resolution and check that all target roots exist (#6480) + `PR #6480 `_ + +Refactoring, Improvements, and Tooling +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* delete unnecessary testproject and broken test (#6494) + `PR #6494 `_ + +* skip integration test with pants_requirement() (#6493) + `PR #6493 `_ + +* Add bootstrapper jar to compile the compile-bridge. (#6462) + `PR #6462 `_ + +* [Hermetic zinc compile] Memoize scalac classpath snapshots (#6491) + `PR #6491 `_ + +* remove FIXME and (cosmicexplorer) comments (#6479) + `PR #6479 `_ + +* Consume the bootstrapper and modify zinc to allow remote exec (#6463) + `PR #6463 `_ + 1.10.0rc0 (09/10/2018) ---------------------- diff --git a/testprojects/src/java/org/pantsbuild/testproject/compilation_warnings/BUILD b/testprojects/src/java/org/pantsbuild/testproject/compilation_warnings/BUILD index 76919d16005..d1c0c22694b 100644 --- a/testprojects/src/java/org/pantsbuild/testproject/compilation_warnings/BUILD +++ b/testprojects/src/java/org/pantsbuild/testproject/compilation_warnings/BUILD @@ -1,16 +1,11 @@ java_library( name='fatal', - fatal_warnings=True, + compiler_option_sets={'fatal_warnings'}, sources=['Warning.java'], ) java_library( name='nonfatal', - fatal_warnings=False, - sources=['Warning.java'], -) - -java_library( - name='defaultfatal', + compiler_option_sets={}, sources=['Warning.java'], ) diff --git a/testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings/BUILD b/testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings/BUILD index 71b527c8ce9..1ad55b148a7 100644 --- a/testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings/BUILD +++ b/testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings/BUILD @@ -1,16 +1,11 @@ scala_library( name='fatal', - fatal_warnings=True, + compiler_option_sets={'fatal_warnings'}, sources=['Warning.scala'], ) scala_library( name='nonfatal', - fatal_warnings=False, - sources=['Warning.scala'], -) - -scala_library( - name='defaultfatal', + compiler_option_sets={}, sources=['Warning.scala'], ) diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py index 03eb9ff6069..695dcce9936 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py @@ -130,29 +130,21 @@ def test_stale_apt_with_deps(self): clean_all=False)) def test_fatal_warning(self): - def test_combination(target, default_fatal_warnings, expect_success): + def test_combination(target, expect_success): with self.temporary_workdir() as workdir: with self.temporary_cachedir() as cachedir: - if default_fatal_warnings: - arg = '--java-fatal-warnings' - else: - arg = '--no-java-fatal-warnings' pants_run = self.run_test_compile( workdir, cachedir, 'testprojects/src/java/org/pantsbuild/testproject/compilation_warnings:{}'.format(target), - extra_args=[arg, '--compile-zinc-warning-args=-C-Xlint:all']) + extra_args=['--compile-zinc-warning-args=-C-Xlint:all']) if expect_success: self.assert_success(pants_run) else: self.assert_failure(pants_run) - test_combination('defaultfatal', default_fatal_warnings=True, expect_success=False) - test_combination('defaultfatal', default_fatal_warnings=False, expect_success=True) - test_combination('fatal', default_fatal_warnings=True, expect_success=False) - test_combination('fatal', default_fatal_warnings=False, expect_success=False) - test_combination('nonfatal', default_fatal_warnings=True, expect_success=True) - test_combination('nonfatal', default_fatal_warnings=False, expect_success=True) + test_combination('fatal', expect_success=False) + test_combination('nonfatal', expect_success=True) def test_classpath_does_not_include_extra_classes_dirs(self): target_rel_spec = 'testprojects/src/java/org/pantsbuild/testproject/phrases:' diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py index 8413668bfcc..515ab9544e1 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py @@ -125,34 +125,26 @@ def test_zinc_unsupported_option(self): self.assertIn('is not supported, and is subject to change/removal', pants_run.stdout_data) def test_zinc_fatal_warning(self): - def test_combination(target, default_fatal_warnings, expect_success, extra_args=[]): + def test_combination(target, expect_success, extra_args=[]): with self.temporary_workdir() as workdir: with self.temporary_cachedir() as cachedir: - if default_fatal_warnings: - arg = '--scala-fatal-warnings' - else: - arg = '--no-scala-fatal-warnings' pants_run = self.run_test_compile( workdir, cachedir, 'testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings:{}'.format( target), - extra_args=[arg] + extra_args) + extra_args=extra_args) if expect_success: self.assert_success(pants_run) else: self.assert_failure(pants_run) - test_combination('defaultfatal', default_fatal_warnings=True, expect_success=False) - test_combination('defaultfatal', default_fatal_warnings=False, expect_success=True) - test_combination('fatal', default_fatal_warnings=True, expect_success=False) - test_combination('fatal', default_fatal_warnings=False, expect_success=False) - test_combination('nonfatal', default_fatal_warnings=True, expect_success=True) - test_combination('nonfatal', default_fatal_warnings=False, expect_success=True) - - test_combination('fatal', default_fatal_warnings=True, expect_success=True, + test_combination('fatal', expect_success=False) + test_combination('nonfatal', expect_success=True) + + test_combination('fatal', expect_success=True, extra_args=['--compile-zinc-fatal-warnings-enabled-args=[\'-C-Werror\']']) - test_combination('fatal', default_fatal_warnings=False, expect_success=False, + test_combination('fatal', expect_success=False, extra_args=['--compile-zinc-fatal-warnings-disabled-args=[\'-S-Xfatal-warnings\']']) @unittest.expectedFailure diff --git a/tests/python/pants_test/backend/jvm/tasks/test_classmap.py b/tests/python/pants_test/backend/jvm/tasks/test_classmap.py index 9baf06e4b27..f62b9aa3404 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_classmap.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_classmap.py @@ -6,11 +6,8 @@ from contextlib import contextmanager -from pants.backend.jvm.targets.jar_library import JarLibrary -from pants.backend.jvm.targets.java_library import JavaLibrary from pants.backend.jvm.tasks.classmap import ClassmapTask from pants.build_graph.target import Target -from pants.java.jar.jar_dependency import JarDependency from pants.util.contextutil import open_zip from pants_test.backend.jvm.tasks.jvm_binary_task_test_base import JvmBinaryTaskTestBase from pants_test.subsystem.subsystem_util import init_subsystem @@ -26,16 +23,28 @@ def setUp(self): super(ClassmapTaskTest, self).setUp() init_subsystem(Target.Arguments) - self.target_a = self.make_target('a', target_type=JavaLibrary, sources=['a1.java', 'a2.java']) + self.add_to_build_file( + 'a', + 'java_library(sources=["a1.java", "a2.java"])', + ) self.jar_artifact = self.create_artifact(org='org.example', name='foo', rev='1.0.0') with open_zip(self.jar_artifact.pants_path, 'w') as jar: jar.writestr('foo/Foo.class', '') - self.target_b = self.make_target('b', target_type=JarLibrary, - jars=[JarDependency(org='org.example', name='foo', rev='1.0.0')]) - self.target_c = self.make_target('c', dependencies=[self.target_a, self.target_b], - target_type=JavaLibrary) + self.add_to_build_file( + 'b', + 'jar_library(jars=[jar(org="org.example", name="foo", rev="1.0.0")])', + ) + + self.add_to_build_file( + 'c', + 'java_library(dependencies=["a", "b"])', + ) + + self.target_a = self.target('a') + self.target_b = self.target('b') + self.target_c = self.target('c') @contextmanager def prepare_context(self, options=None): diff --git a/tests/python/pants_test/backend/jvm/tasks/test_prepare_services.py b/tests/python/pants_test/backend/jvm/tasks/test_prepare_services.py index 9aa13d81696..8cf11513724 100644 --- a/tests/python/pants_test/backend/jvm/tasks/test_prepare_services.py +++ b/tests/python/pants_test/backend/jvm/tasks/test_prepare_services.py @@ -10,6 +10,7 @@ from pants.backend.jvm.targets.java_library import JavaLibrary from pants.backend.jvm.targets.jvm_target import JvmTarget from pants.backend.jvm.tasks.prepare_services import PrepareServices +from pants.build_graph.build_file_aliases import BuildFileAliases from pants.java.jar.exclude import Exclude from pants.util.contextutil import temporary_dir from pants_test.task_test_base import TaskTestBase @@ -20,6 +21,13 @@ class PrepareServicesTest(TaskTestBase): def task_type(cls): return PrepareServices + @classmethod + def alias_groups(cls): + """ + :API: public + """ + return BuildFileAliases(targets={'java_library': JavaLibrary}) + def test_find_all_relevant_resources_targets(self): jvm_target = self.make_target('jvm:target', target_type=JvmTarget) java_library = self.make_target('java:target', target_type=JavaLibrary, sources=[]) @@ -97,11 +105,20 @@ def assert_no_resources_prepared(target): def test_prepare_resources(self): task = self.create_task(self.context()) - target = self.make_target('java:target', - target_type=JavaLibrary, - services={'ServiceInterfaceA': ['ServiceImplA1, ServiceImplA2'], - 'ServiceInterfaceB': [], - 'ServiceInterfaceC': ['ServiceImplC1']}) + self.add_to_build_file( + 'java', + ''' +java_library( + name = "target", + services = { + "ServiceInterfaceA": ["ServiceImplA1, ServiceImplA2"], + "ServiceInterfaceB": [], + "ServiceInterfaceC": ["ServiceImplC1"], + }, +) +''' + ) + target = self.target('java:target') with temporary_dir() as chroot: task.prepare_resources(target, chroot) resource_files = {} diff --git a/tests/python/pants_test/build_graph/test_target.py b/tests/python/pants_test/build_graph/test_target.py index 8b5d5e86b8b..d5abfe82b5e 100644 --- a/tests/python/pants_test/build_graph/test_target.py +++ b/tests/python/pants_test/build_graph/test_target.py @@ -129,28 +129,6 @@ def test_target_id_short(self): self.assertEqual(short_id, 'dummy.dummy1.dummy2.dummy3.dummy4.dummy5.dummy6.dummy7.dummy8.dummy9.foo') - def test_implicit_sources(self): - options = {Target.Arguments.options_scope: {'implicit_sources': True}} - init_subsystem(Target.Arguments, options) - target = self.make_target(':a', ImplicitSourcesTestingTarget) - # Note explicit key_arg. - sources = target.create_sources_field(sources=None, sources_rel_path='src/foo/bar', - key_arg='sources') - self.assertEqual(sources.filespec, {'globs': ['src/foo/bar/*.foo']}) - - target = self.make_target(':b', ImplicitSourcesTestingTargetMulti) - # Note no explicit key_arg, which should behave just like key_arg='sources'. - sources = target.create_sources_field(sources=None, sources_rel_path='src/foo/bar') - self.assertEqual(sources.filespec, { - 'globs': ['src/foo/bar/*.foo', 'src/foo/bar/*.bar'], - 'exclude': [{'globs': ['src/foo/bar/*.baz', 'src/foo/bar/*.qux']}], - }) - - # Ensure that we don't use implicit sources when creating resources fields. - resources = target.create_sources_field(sources=None, sources_rel_path='src/foo/bar', - key_arg='resources') - self.assertEqual(resources.filespec, {'globs': []}) - def test_create_sources_field_with_string_fails(self): target = self.make_target(':a-target', Target) diff --git a/tests/python/pants_test/task/test_task.py b/tests/python/pants_test/task/test_task.py index 34644132179..7fc60d00e80 100644 --- a/tests/python/pants_test/task/test_task.py +++ b/tests/python/pants_test/task/test_task.py @@ -11,6 +11,7 @@ from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError +from pants.build_graph.build_file_aliases import BuildFileAliases from pants.build_graph.files import Files from pants.cache.cache_setup import CacheSetup from pants.option.arg_splitter import GLOBAL_SCOPE @@ -137,6 +138,13 @@ class TaskTest(TaskTestBase): def task_type(cls): return DummyTask + @classmethod + def alias_groups(cls): + """ + :API: public + """ + return BuildFileAliases(targets={'files': Files}) + def assertContent(self, vt, content): with open(os.path.join(vt.current_results_dir, self._filename), 'r') as f: self.assertEqual(f.read(), content) @@ -151,30 +159,39 @@ def _toggle_cache(self, enable_artifact_cache): read=enable_artifact_cache, ) - def _fixture(self, incremental, options=None): - target = self.make_target( - ':t', - target_type=Files, - sources=[self._filename], - make_missing_sources=False, + def _write_build_file(self): + self.add_to_build_file( + '', + ''' +files( + name = "t", + sources = ["{filename}"], +) +'''.format(filename=self._filename) ) + + def _task(self, incremental, options=None): + target = self.target(':t') context = self.context(options=options, target_roots=[target]) task = self.create_task(context) task._incremental = incremental - return task, target + return task + + def _run_fixture(self, content=None, incremental=False, artifact_cache=False, options=None, write_build_file=True): + if write_build_file: + self._write_build_file() - def _run_fixture(self, content=None, incremental=False, artifact_cache=False, options=None): content = content or self._file_contents self._toggle_cache(artifact_cache) - task, target = self._fixture(incremental=incremental, options=options) - self._create_clean_file(target, content) + self._create_clean_file(content) + task = self._task(incremental=incremental, options=options) vtA, was_valid = task.execute() return task, vtA, was_valid - def _create_clean_file(self, target, content): + def _create_clean_file(self, content): self.create_file(self._filename, content) - target.mark_invalidation_hash_dirty() + self.reset_build_graph() def _cache_ignore_options(self, globally=False): return { @@ -222,23 +239,27 @@ def test_revert_after_failure(self): good_content = "good_content" bad_content = "bad_content" - task, target = self._fixture(incremental=False) + + self._write_build_file() # Clean run succeeds. - self._create_clean_file(target, good_content) + self._create_clean_file(good_content) + task = self._task(incremental=False) vt, was_valid = task.execute() self.assertFalse(was_valid) self.assertContent(vt, good_content) # Change causes the task to fail. - self._create_clean_file(target, bad_content) + self._create_clean_file(bad_content) + task = self._task(incremental=False) task._force_fail = True self.assertRaises(TaskError, task.execute) task._force_fail = False # Reverting to the previous content should invalidate, so the task # can reset any state created by the failed run. - self._create_clean_file(target, good_content) + self._create_clean_file(good_content) + task = self._task(incremental=False) vt, was_valid = task.execute() self.assertFalse(was_valid) self.assertContent(vt, good_content) @@ -246,19 +267,22 @@ def test_revert_after_failure(self): def test_incremental(self): """Run three times with two unique fingerprints.""" + self._write_build_file() + one = '1\n' two = '2\n' three = '3\n' - task, target = self._fixture(incremental=True) # Clean - this is the first run so the VT is invalid. - self._create_clean_file(target, one) + self._create_clean_file(one) + task = self._task(incremental=True) vtA, was_A_valid = task.execute() self.assertFalse(was_A_valid) self.assertContent(vtA, one) # Changed the source file, so it copies the results from vtA. - self._create_clean_file(target, two) + self._create_clean_file(two) + task = self._task(incremental=True) vtB, was_B_valid = task.execute() self.assertFalse(was_B_valid) self.assertEqual(vtB.previous_cache_key, vtA.cache_key) @@ -266,7 +290,8 @@ def test_incremental(self): self.assertTrue(vtB.has_previous_results_dir) # Another changed source means a new cache_key. The previous_results_dir is copied. - self._create_clean_file(target, three) + self._create_clean_file(three) + task = self._task(incremental=True) vtC, was_C_valid = task.execute() self.assertFalse(was_C_valid) self.assertTrue(vtC.has_previous_results_dir) @@ -274,11 +299,12 @@ def test_incremental(self): self.assertContent(vtC, one + two + three) # Again create a clean file but this time reuse an old cache key - in this case vtB. - self._create_clean_file(target, two) + self._create_clean_file(two) # This VT will be invalid, since there is no cache hit and it doesn't match the immediately # previous run. It will wipe the invalid vtB.current_results_dir and followup by copying in the # most recent results_dir, from vtC. + task = self._task(incremental=True) vtD, was_D_valid = task.execute() self.assertFalse(was_D_valid) self.assertTrue(vtD.has_previous_results_dir) @@ -292,15 +318,18 @@ def test_incremental(self): def test_non_incremental(self): """Non-incremental should be completely unassociated.""" + self._write_build_file() + one = '1\n' two = '2\n' - task, target = self._fixture(incremental=False) # Run twice. - self._create_clean_file(target, one) + self._create_clean_file(one) + task = self._task(incremental=False) vtA, _ = task.execute() self.assertContent(vtA, one) - self._create_clean_file(target, two) + self._create_clean_file(two) + task = self._task(incremental=False) vtB, _ = task.execute() # Confirm two unassociated current directories with a stable results_dir. @@ -312,17 +341,20 @@ def test_non_incremental(self): def test_implementation_version(self): """When the implementation version changes, previous artifacts are not available.""" + self._write_build_file() + one = '1\n' two = '2\n' - task, target = self._fixture(incremental=True) # Run twice, with a different implementation version the second time. DummyTask._implementation_version = 0 - self._create_clean_file(target, one) + self._create_clean_file(one) + task = self._task(incremental=True) vtA, _ = task.execute() self.assertContent(vtA, one) DummyTask._implementation_version = 1 - self._create_clean_file(target, two) + self._create_clean_file(two) + task = self._task(incremental=True) vtB, _ = task.execute() # No incrementalism. @@ -376,7 +408,9 @@ def test_cache_hit_short_circuits_incremental_copy(self): # This results in an invalid vt, with no cache hit. It will then copy the vtB.previous_results # into vtC.results_dir. self._toggle_cache(False) - self._create_clean_file(vtB.target, second_contents) + self._create_clean_file(second_contents) + + task = self._task(incremental=True) vtC, was_C_valid = task.execute() self.assertNotEqual(vtB.cache_key.hash, vtC.cache_key.hash) @@ -401,7 +435,8 @@ def test_live_dirs(self): self.assertIn(vtA.current_results_dir, vtA_live) self.assertEqual(len(vtA_live), 2) - self._create_clean_file(vtA.target, 'bar') + self._create_clean_file('bar') + task = self._task(incremental=True) vtB, _ = task.execute() vtB_live = list(vtB.live_dirs()) @@ -415,7 +450,8 @@ def test_live_dirs(self): # previous_cache_key. safe_rmtree(vtB.current_results_dir) - self._create_clean_file(vtB.target, 'baz') + self._create_clean_file('baz') + task = self._task(incremental=True) vtC, _ = task.execute() vtC_live = list(vtC.live_dirs()) self.assertNotIn(vtB.current_results_dir, vtC_live) @@ -427,12 +463,15 @@ def test_ignore_global(self): self.assertTrue(vtA.cacheable) self.reset_build_graph() - _, vtA, was_valid = self._run_fixture() + _, vtA, was_valid = self._run_fixture(write_build_file=False) self.assertTrue(was_valid) self.assertTrue(vtA.cacheable) self.reset_build_graph() - _, vtA, was_valid = self._run_fixture(options=self._cache_ignore_options(globally=True)) + _, vtA, was_valid = self._run_fixture( + options=self._cache_ignore_options(globally=True), + write_build_file=False, + ) self.assertFalse(was_valid) self.assertFalse(vtA.cacheable) @@ -442,12 +481,15 @@ def test_ignore(self): self.assertTrue(vtA.cacheable) self.reset_build_graph() - _, vtA, was_valid = self._run_fixture() + _, vtA, was_valid = self._run_fixture(write_build_file=False) self.assertTrue(was_valid) self.assertTrue(vtA.cacheable) self.reset_build_graph() - _, vtA, was_valid = self._run_fixture(options=self._cache_ignore_options()) + _, vtA, was_valid = self._run_fixture( + options=self._cache_ignore_options(), + write_build_file=False, + ) self.assertFalse(was_valid) self.assertFalse(vtA.cacheable)