Fix Go source excludes; Cleanup old filespec matching

contrib/go/src/python/pants/contrib/go/targets/go_local_source.py

@@ -67,8 +67,8 @@ def __init__(self, address=None, payload=None, **kwargs):
     #    for example via plain old filesystem access.
     globs = Globs(ParseContext(rel_path=address.spec_path, type_aliases={}))
     sources = globs('*', exclude=[globs('BUILD*'),
-                                  # This skips dirents.
-                                  globs('*/')])
+                                  # skip subdir contents
+                                  globs('*/**')])
 
     payload = payload or Payload()
     payload.add_fields({

src/python/pants/build_graph/source_mapper.py

@@ -58,7 +58,7 @@ def _find_targets_for_source(self, source, spec_path):
       self._build_graph.inject_address_closure(address)
       target = self._build_graph.get_target(address)
       sources = target.payload.get_field('sources')
-      if self._sources_match(source, sources):
+      if sources and sources.matches(source):
         yield address
       elif self._address_mapper.is_declaring_file(address, source):
         yield address
@@ -71,11 +71,6 @@ def _find_targets_for_source(self, source, spec_path):
             yield address
             break
 
-  def _sources_match(self, source, sources):
-    if not sources:
-      return False
-    return sources.matches(source)
-
 
 class LazySourceMapper(SourceMapper):
   """

src/python/pants/source/filespec.py

@@ -16,7 +16,7 @@ def glob_to_regex(pattern):
   :returns: A regex string that matches same paths as the input glob does.
   """
   out = ['^']
-  components = pattern.strip('/').replace('.', '[.]').split('/')
+  components = pattern.strip('/').replace('.', '[.]').replace('$', '[$]').split('/')
   doublestar = False
   for component in components:
     if len(out) == 1:

src/python/pants/source/wrapped_globs.py

@@ -6,13 +6,14 @@
                         unicode_literals, with_statement)
 
 import os
-from abc import abstractproperty
+from abc import abstractmethod, abstractproperty
 from hashlib import sha1
 
 from six import string_types
 from twitter.common.dirutil.fileset import Fileset
 
 from pants.base.build_environment import get_buildroot
+from pants.source.filespec import matches_filespec
 from pants.util.dirutil import fast_relpath
 from pants.util.memo import memoized_property
 from pants.util.meta import AbstractClass
@@ -64,18 +65,17 @@ def __iter__(self):
   def __getitem__(self, index):
     return self.files[index]
 
+  @abstractmethod
   def iter_relative_paths(self):
     """An alternative `__iter__` that joins files with the relative root."""
-    for f in self:
-      yield os.path.join(self.rel_root, f)
 
 
 class EagerFilesetWithSpec(FilesetWithSpec):
   def __init__(self, rel_root, filespec, files, files_hash):
     """
     :param rel_root: The root for the given filespec, relative to the buildroot.
     :param filespec: A filespec as generated by `FilesetRelPathWrapper`, which represents
-      what globs or file list it came from.
+      what globs or file list it came from. Must be relative to buildroot.
     :param files: A list of matched files, with declared order and duplicates preserved.
     :param files_hash: A string fingerprint for all files in the fileset.
     """
@@ -91,6 +91,11 @@ def files(self):
   def files_hash(self):
     return self._files_hash
 
+  def iter_relative_paths(self):
+    """An alternative `__iter__` that joins files with the relative root."""
+    for f in self:
+      yield os.path.join(self.rel_root, f)
+
   def __repr__(self):
     return 'EagerFilesetWithSpec(rel_root={!r}, files={!r})'.format(self.rel_root, self.files)
 
@@ -120,6 +125,13 @@ def files_hash(self):
         h.update(f.read())
     return h.digest()
 
+  def iter_relative_paths(self):
+    """An alternative `__iter__` that joins files with the relative root."""
+    for f in self:
+      path_from_buildroot = os.path.join(self.rel_root, f)
+      if matches_filespec(path_from_buildroot, self.filespec):
+        yield path_from_buildroot
+
 
 class FilesetRelPathWrapper(AbstractClass):
   KNOWN_PARAMETERS = frozenset(['exclude', 'follow_links'])

tests/python/pants_test/source/test_filespec.py

@@ -23,6 +23,9 @@ def assert_rule_match(self, glob, expected_matches, negate=False):
       asserter(re.match(regex, expected), 'glob_to_regex(`{}`) -> `{}` {} path `{}`'
                                           .format(glob, regex, match_state, expected))
 
+  def test_glob_to_regex_with_dollar_sign(self):
+    self.assert_rule_match('a/b$abc*.py', ('a/b$abc.py', ))
+
   def test_glob_to_regex_single_star_0(self):
     self.assert_rule_match('a/b/*/f.py', ('a/b/c/f.py', 'a/b/q/f.py'))
 
@@ -74,6 +77,10 @@ def test_glob_to_regex_leading_slash_1_neg(self):
   def test_glob_to_regex_leading_slash_2(self):
     self.assert_rule_match('/**', ('/a', '/a/b/c/d/e/f'))
 
+  def test_glob_to_regex_match_subdir(self):
+    self.assert_rule_match('*/**', ('a/b.go', 'a/b/c.go', 'a/b/c/d.go'))
+    self.assert_rule_match('*/**', ('a.go'), negate=True)
+
   def test_glob_to_regex_leading_slash_2_neg(self):
     self.assert_rule_match('/**', ('a', 'a/b/c/d/e/f'), negate=True)
 

tests/python/pants_test/source/test_payload_fields.py

@@ -71,7 +71,12 @@ def test_passes_lazy_fileset_with_spec_through(self):
   def test_passes_eager_fileset_with_spec_through(self):
     self.create_file('foo/a.txt', 'a_contents')
 
-    fileset = EagerFilesetWithSpec('foo', {'globs': ['foo/a.txt']}, ['foo/a.txt'], {'foo/a.txt': b'12345'})
+    fileset = EagerFilesetWithSpec(rel_root='foo',
+                                   # Glob spec is relative to build root
+                                   filespec={'globs': ['foo/foo/a.txt']},
+                                   # files are relative to `rel_root`
+                                   files=['foo/a.txt'],
+                                   files_hash={'foo/a.txt': b'12345'})
     sf = SourcesField(sources=fileset)
 
     self.assertIs(fileset, sf.sources)