diff --git a/examples/src/wire/org/pantsbuild/example/element/BUILD b/examples/src/wire/org/pantsbuild/example/element/BUILD index 916a7f943a1..0afde70a649 100644 --- a/examples/src/wire/org/pantsbuild/example/element/BUILD +++ b/examples/src/wire/org/pantsbuild/example/element/BUILD @@ -3,10 +3,12 @@ java_wire_library( sources=[ - 'elements.proto', # Order matters here. + # NB: Order matters for these two paths, so we set `ordered_sources=True` below. + 'elements.proto', 'compound.proto', ], dependencies=[ 'examples/src/wire/org/pantsbuild/example/temperature', ], + ordered_sources=True, ) diff --git a/src/python/pants/backend/codegen/wire/java/java_wire_library.py b/src/python/pants/backend/codegen/wire/java/java_wire_library.py index d5cbd9b3fbe..93391af49c4 100644 --- a/src/python/pants/backend/codegen/wire/java/java_wire_library.py +++ b/src/python/pants/backend/codegen/wire/java/java_wire_library.py @@ -32,6 +32,7 @@ def __init__(self, registry_class=None, enum_options=None, no_options=None, + ordered_sources=None, **kwargs): """ :param string service_writer: the name of the class to pass as the --service_writer option to @@ -43,6 +44,9 @@ def __init__(self, doubt, specify com.squareup.wire.SimpleServiceWriter :param list enum_options: list of enums to pass to as the --enum-enum_options option, # optional :param boolean no_options: boolean that determines if --no_options flag is passed + :param boolean ordered_sources: boolean that declares whether the sources argument represents + literal ordered sources to be passed directly to the compiler. If false, no ordering is + guaranteed for the sources passed to an individual compiler invoke. """ if not service_writer and service_writer_options: @@ -59,6 +63,7 @@ def __init__(self, 'registry_class': PrimitiveField(registry_class or None), 'enum_options': PrimitiveField(enum_options or []), 'no_options': PrimitiveField(no_options or False), + 'ordered_sources': PrimitiveField(ordered_sources or False), }) super(JavaWireLibrary, self).__init__(payload=payload, **kwargs) diff --git a/src/python/pants/backend/codegen/wire/java/wire_gen.py b/src/python/pants/backend/codegen/wire/java/wire_gen.py index 84b378d9136..acfb8d58f11 100644 --- a/src/python/pants/backend/codegen/wire/java/wire_gen.py +++ b/src/python/pants/backend/codegen/wire/java/wire_gen.py @@ -13,10 +13,12 @@ from pants.backend.jvm.targets.java_library import JavaLibrary from pants.backend.jvm.tasks.nailgun_task import NailgunTaskBase from pants.base.build_environment import get_buildroot -from pants.base.exceptions import TaskError +from pants.base.exceptions import TargetDefinitionException, TaskError from pants.base.workunit import WorkUnitLabel from pants.java.jar.jar_dependency import JarDependency +from pants.source.filespec import globs_matches from pants.task.simple_codegen_task import SimpleCodegenTask +from pants.util.dirutil import fast_relpath logger = logging.getLogger(__name__) @@ -61,24 +63,47 @@ def synthetic_target_extra_dependencies(self, target, target_workdir): wire_runtime_deps_spec = self.get_options().javadeps return self.resolve_deps([wire_runtime_deps_spec]) - def format_args_for_target(self, target, target_workdir): - """Calculate the arguments to pass to the command line for a single target.""" - sources = OrderedSet(target.sources_relative_to_buildroot()) - + def _compute_sources(self, target): relative_sources = OrderedSet() - source_roots = set() - for source in sources: + source_roots = OrderedSet() + + def capture_and_relativize_to_source_root(source): source_root = self.context.source_roots.find_by_path(source) if not source_root: source_root = self.context.source_roots.find(target) source_roots.add(source_root.path) - relative_source = os.path.relpath(source, source_root.path) - relative_sources.add(relative_source) + return fast_relpath(source, source_root.path) + + if target.payload.get_field_value('ordered_sources'): + # Re-match the filespecs against the sources in order to apply them in the literal order + # they were specified in. + filespec = target.globs_relative_to_buildroot() + excludes = filespec.get('excludes', []) + for filespec in filespec.get('globs', []): + sources = [s for s in target.sources_relative_to_buildroot() + if globs_matches([s], [filespec], excludes)] + if len(sources) != 1: + raise TargetDefinitionException( + target, + 'With `ordered_sources=True`, expected one match for each file literal, ' + 'but got: {} for literal `{}`.'.format(sources, filespec) + ) + relative_sources.add(capture_and_relativize_to_source_root(sources[0])) + else: + # Otherwise, use the default (unspecified) snapshot ordering. + for source in target.sources_relative_to_buildroot(): + relative_sources.add(capture_and_relativize_to_source_root(source)) + return relative_sources, source_roots + + def format_args_for_target(self, target, target_workdir): + """Calculate the arguments to pass to the command line for a single target.""" args = ['--java_out={0}'.format(target_workdir)] # Add all params in payload to args + relative_sources, source_roots = self._compute_sources(target) + if target.payload.get_field_value('no_options'): args.append('--no_options') diff --git a/src/python/pants/source/filespec.py b/src/python/pants/source/filespec.py index c36d37176a4..77e967043f5 100644 --- a/src/python/pants/source/filespec.py +++ b/src/python/pants/source/filespec.py @@ -9,6 +9,9 @@ def glob_to_regex(pattern): """Given a glob pattern, return an equivalent regex expression. + + TODO: Replace with implementation in `fs.rs`. See https://github.com/pantsbuild/pants/issues/6795. + :param string glob: The glob pattern. "**" matches 0 or more dirs recursively. "*" only matches patterns in a single dir. :returns: A regex string that matches same paths as the input glob does. diff --git a/src/rust/engine/fs/src/snapshot.rs b/src/rust/engine/fs/src/snapshot.rs index eaa1eca90da..3701fd2278e 100644 --- a/src/rust/engine/fs/src/snapshot.rs +++ b/src/rust/engine/fs/src/snapshot.rs @@ -44,23 +44,23 @@ impl Snapshot { >( store: Store, file_digester: &S, - path_stats: Vec, + mut path_stats: Vec, ) -> BoxFuture { - let mut sorted_path_stats = path_stats.clone(); - sorted_path_stats.sort_by(|a, b| a.path().cmp(b.path())); + path_stats.sort_by(|a, b| a.path().cmp(b.path())); // The helper assumes that if a Path has multiple children, it must be a directory. // Proactively error if we run into identically named files, because otherwise we will treat // them like empty directories. - sorted_path_stats.dedup_by(|a, b| a.path() == b.path()); - if sorted_path_stats.len() != path_stats.len() { + let pre_dedupe_len = path_stats.len(); + path_stats.dedup_by(|a, b| a.path() == b.path()); + if path_stats.len() != pre_dedupe_len { return future::err(format!( "Snapshots must be constructed from unique path stats; got duplicates in {:?}", path_stats )) .to_boxed(); } - Snapshot::ingest_directory_from_sorted_path_stats(store, file_digester, &sorted_path_stats) + Snapshot::ingest_directory_from_sorted_path_stats(store, file_digester, &path_stats) .map(|digest| Snapshot { digest, path_stats }) .to_boxed() } @@ -552,6 +552,27 @@ mod tests { ); } + #[test] + fn snapshot_from_digest() { + let (store, dir, posix_fs, digester) = setup(); + + let cats = PathBuf::from("cats"); + let roland = cats.join("roland"); + std::fs::create_dir_all(&dir.path().join(cats)).unwrap(); + make_file(&dir.path().join(&roland), STR.as_bytes(), 0o600); + + let path_stats = expand_all_sorted(posix_fs); + let expected_snapshot = Snapshot::from_path_stats(store.clone(), &digester, path_stats.clone()) + .wait() + .unwrap(); + assert_eq!( + expected_snapshot, + Snapshot::from_digest(store, expected_snapshot.digest) + .wait() + .unwrap(), + ); + } + #[test] fn snapshot_recursive_directories_including_empty() { let (store, dir, posix_fs, digester) = setup(); @@ -580,7 +601,7 @@ mod tests { .unwrap(), 232, ), - path_stats: unsorted_path_stats, + path_stats: sorted_path_stats, } ); } diff --git a/tests/python/pants_test/backend/codegen/protobuf/java/BUILD b/tests/python/pants_test/backend/codegen/protobuf/java/BUILD index da9c887c158..90451d248da 100644 --- a/tests/python/pants_test/backend/codegen/protobuf/java/BUILD +++ b/tests/python/pants_test/backend/codegen/protobuf/java/BUILD @@ -26,4 +26,5 @@ python_tests( 'tests/python/pants_test:int-test', ], tags = {'integration'}, + timeout = 240, ) diff --git a/tests/python/pants_test/backend/codegen/protobuf/java/test_protobuf_integration.py b/tests/python/pants_test/backend/codegen/protobuf/java/test_protobuf_integration.py index 4c1c944cfa7..4075f0ea228 100644 --- a/tests/python/pants_test/backend/codegen/protobuf/java/test_protobuf_integration.py +++ b/tests/python/pants_test/backend/codegen/protobuf/java/test_protobuf_integration.py @@ -112,9 +112,7 @@ def find_protoc_blocks(lines): out=pants_run.stderr_data, blocks=block_text)) - biggest_proto = -1 for block in all_blocks: - last_proto = -1 seen_extracted = False for line in block: # Make sure import bases appear after the bases for actual sources. @@ -124,14 +122,3 @@ def find_protoc_blocks(lines): else: self.assertFalse(seen_extracted, 'Local protoc bases must be ordered before imported bases!') - continue - # Check to make sure, eg, testproto4.proto never precedes testproto2.proto. - match = re.search(r'(?P\d+)\.proto[\\.]?$', line) - if match: - number = int(match.group('sequence')) - self.assertTrue(number > last_proto, '{proto} succeeded proto #{number}!\n{blocks}' - .format(proto=line, number=last_proto, blocks=block_text)) - last_proto = number - if last_proto > biggest_proto: - biggest_proto = last_proto - self.assertEqual(biggest_proto, 6, 'Not all protos were seen!\n{}'.format(block_text)) diff --git a/tests/python/pants_test/backend/codegen/wire/java/BUILD b/tests/python/pants_test/backend/codegen/wire/java/BUILD index 99e8abec771..03c82fe8d84 100644 --- a/tests/python/pants_test/backend/codegen/wire/java/BUILD +++ b/tests/python/pants_test/backend/codegen/wire/java/BUILD @@ -6,6 +6,7 @@ python_tests( sources = globs('*.py', exclude=[globs('*_integration.py')]), dependencies = [ '3rdparty/python/twitter/commons:twitter.common.collections', + '3rdparty/python:parameterized', 'src/python/pants/backend/codegen/wire/java', 'src/python/pants/java/jar', 'src/python/pants/backend/jvm/targets:jvm', @@ -29,4 +30,5 @@ python_tests( 'tests/python/pants_test:int-test', ], tags = {'integration'}, + timeout = 300, ) diff --git a/tests/python/pants_test/backend/codegen/wire/java/test_wire_gen.py b/tests/python/pants_test/backend/codegen/wire/java/test_wire_gen.py index e05d37b0f4e..b297af56110 100644 --- a/tests/python/pants_test/backend/codegen/wire/java/test_wire_gen.py +++ b/tests/python/pants_test/backend/codegen/wire/java/test_wire_gen.py @@ -4,6 +4,8 @@ from __future__ import absolute_import, division, print_function, unicode_literals +from parameterized import parameterized + from pants.backend.codegen.wire.java.java_wire_library import JavaWireLibrary from pants.backend.codegen.wire.java.register import build_file_aliases as register_codegen from pants.backend.codegen.wire.java.wire_gen import WireGen @@ -59,28 +61,35 @@ def test_compiler_args_wirev1(self): 'bar.proto'], task.format_args_for_target(wire_targetv1, self.TARGET_WORKDIR)) - def test_compiler_args_all(self): + @parameterized.expand([(True,), (False,)]) + def test_compiler_args_all(self, ordered_sources): self._create_fake_wire_tool(version='1.8.0') kitchen_sink = self.make_target('src/wire:kitchen-sink', JavaWireLibrary, sources=['foo.proto', 'bar.proto', 'baz.proto'], registry_class='org.pantsbuild.Registry', service_writer='org.pantsbuild.DummyServiceWriter', no_options=True, + ordered_sources=ordered_sources, roots=['root1', 'root2', 'root3'], enum_options=['enum1', 'enum2', 'enum3'],) task = self.create_task(self.context(target_roots=[kitchen_sink])) - self.assertEqual([ - '--java_out={}'.format(self.TARGET_WORKDIR), - '--no_options', - '--service_writer=org.pantsbuild.DummyServiceWriter', - '--registry_class=org.pantsbuild.Registry', - '--roots=root1,root2,root3', - '--enum_options=enum1,enum2,enum3', - '--proto_path={}/src/wire'.format(self.build_root), - 'foo.proto', - 'bar.proto', - 'baz.proto'], - task.format_args_for_target(kitchen_sink, self.TARGET_WORKDIR)) + expected = [ + '--java_out={}'.format(self.TARGET_WORKDIR), + '--no_options', + '--service_writer=org.pantsbuild.DummyServiceWriter', + '--registry_class=org.pantsbuild.Registry', + '--roots=root1,root2,root3', + '--enum_options=enum1,enum2,enum3', + '--proto_path={}/src/wire'.format(self.build_root), + 'foo.proto', + 'bar.proto', + 'baz.proto', + ] + actual = task.format_args_for_target(kitchen_sink, self.TARGET_WORKDIR) + if not ordered_sources: + expected = set(expected) + actual = set(actual) + self.assertEqual(expected, actual) def test_compiler_args_proto_paths(self): self._create_fake_wire_tool() diff --git a/tests/python/pants_test/engine/legacy/test_graph.py b/tests/python/pants_test/engine/legacy/test_graph.py index 959612a9d63..0ca73b013bc 100644 --- a/tests/python/pants_test/engine/legacy/test_graph.py +++ b/tests/python/pants_test/engine/legacy/test_graph.py @@ -61,8 +61,9 @@ def test_invalidate_fsnode(self): self.assertGreater(invalidated_count, 0) def test_sources_ordering(self): - expected_sources = ['p', 'a', 'n', 't', 's', 'b', 'u', 'i', 'l', 'd'] - self.create_library('src/example', 'files', 'things', sources=expected_sources) + input_sources = ['p', 'a', 'n', 't', 's', 'b', 'u', 'i', 'l', 'd'] + expected_sources = sorted(input_sources) + self.create_library('src/example', 'files', 'things', sources=input_sources) target = self.target('src/example:things') sources = [os.path.basename(s) for s in target.sources_relative_to_buildroot()]