Remove literal-order-preservation from Snapshot, and add a facility t…

…o `java_wire_library` to restore it. Fixes #5802.

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,
 )

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)

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')
 

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.

src/rust/engine/fs/src/snapshot.rs

@@ -44,23 +44,23 @@ impl Snapshot {
   >(
     store: Store,
     file_digester: &S,
-    path_stats: Vec<PathStat>,
+    mut path_stats: Vec<PathStat>,
   ) -> BoxFuture<Snapshot, String> {
-    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,
       }
     );
   }

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,
 )

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<sequence>\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))

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,
 )

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()

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()]