Fix SourceRoots.all_roots to respect fixed roots.

Previously a fixed root that prefix-matched a pattern would lose to the
pattern.

Testing Done:
Modified the existing test to fail then fixed.  Green locally:
```
./pants test \
  tests/python/pants_test/source/:: \
  contrib/go/tests:: \
  contrib/go/examples:: -- -v
```

CI went green here:
  http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3425/6/

Bugs closed: 3421, 3425

Reviewed at https://rbcommons.com/s/twitter/r/3881/

src/python/pants/source/source_root.py

@@ -12,12 +12,37 @@
 
 from pants.base.build_environment import get_buildroot
 from pants.subsystem.subsystem import Subsystem
-from pants.util.memo import memoized_method
+from pants.util.memo import memoized_method, memoized_property
 
 
 SourceRoot = namedtuple('_SourceRoot', ['path', 'langs'])
 
 
+class SourceRootFactory(object):
+  """Creates source roots that respect language canonicalizations."""
+
+  def __init__(self, lang_canonicalizations):
+    """Creates a source root factory that enforces the given `lang_canonicalizations`.
+
+    :param dict lang_canonicalizations: a mapping from language nicknames to the canonical language
+                                        names the nickname could represent.
+    """
+    self._lang_canonicalizations = lang_canonicalizations
+
+  def _canonicalize_langs(self, langs):
+    for lang in (langs or ()):
+      canonicalized = self._lang_canonicalizations.get(lang, (lang,))
+      for canonical in canonicalized:
+        yield canonical
+
+  def create(self, relpath, langs):
+    """Return a source root at the given `relpath` for the given `langs`.
+
+    :returns: :class:`SourceRoot`.
+    """
+    return SourceRoot(relpath, tuple(self._canonicalize_langs(langs)))
+
+
 class SourceRoots(object):
   """An interface for querying source roots."""
 
@@ -29,6 +54,7 @@ def __init__(self, source_root_config):
     Non-test code should not instantiate directly. See SourceRootConfig.get_source_roots().
     """
     self._trie = source_root_config.create_trie()
+    self._source_root_factory = source_root_config.source_root_factory
     self._options = source_root_config.get_options()
 
   def add_source_root(self, path, langs=tuple()):
@@ -83,14 +109,23 @@ def all_roots(self):
     ignore = {'.git'}.union({os.path.relpath(self._options[k], buildroot) for k in
                             ['pants_workdir', 'pants_supportdir', 'pants_distdir']})
 
+    fixed_roots = set()
+    for fixed in self._options.source_roots, self._options.test_roots:
+      if fixed:
+        for root, langs in fixed.items():
+          if os.path.exists(os.path.join(buildroot, root)):
+            yield self._source_root_factory.create(root, langs)
+        fixed_roots.update(fixed.keys())
+
     for dirpath, dirnames, _ in os.walk(buildroot, topdown=True):
       relpath = os.path.relpath(dirpath, buildroot)
       if relpath in ignore:
         del dirnames[:]  # Don't descend into ignored dirs.
       else:
         match = self._trie.find(relpath)
         if match:
-          yield match  # Found a source root.
+          if not any(fixed_root.startswith(relpath) for fixed_root in fixed_roots):
+            yield match  # Found a source root not a prefix of any fixed roots.
           del dirnames[:]  # Don't continue to walk into it.
 
 
@@ -157,11 +192,11 @@ class SourceRootConfig(Subsystem):
     # Go requires some special-case handling of source roots.  In particular, go buildgen assumes
     # that there's a single source root for local code and (optionally) a single source root
     # for remote code.  This fixed source root shows how to capture that distinction.
-    # Go repos may need to add their own appropriate special cases in their pants.ini, until we fix this hack.
+    # Go repos may need to add their own appropriate special cases in their pants.ini, until we fix
+    # this hack.
     # TODO: Treat third-party/remote code as a separate category (akin to 'source' and 'test').
     # Then this hack won't be necessary.
-    '3rdparty/go': ('go_remote', ),
-    'contrib/go/examples/3rdparty/go': ('go_remote', )
+    '3rdparty/go': ('go_remote',),
   }
 
   _DEFAULT_TEST_ROOTS = {
@@ -200,9 +235,12 @@ def get_source_roots(self):
     return SourceRoots(self)
 
   def create_trie(self):
-    """Create a trie of source root patterns from options."""
+    """Create a trie of source root patterns from options.
+
+    :returns: :class:`SourceRootTrie`
+    """
+    trie = SourceRootTrie(self.source_root_factory)
     options = self.get_options()
-    trie = SourceRootTrie(options.lang_canonicalizations)
 
     # Add patterns.
     for pattern in options.source_root_patterns or []:
@@ -218,6 +256,14 @@ def create_trie(self):
 
     return trie
 
+  @memoized_property
+  def source_root_factory(self):
+    """Creates source roots that respects language canonicalizations.
+
+    :returns: :class:`SourceRootFactory`
+    """
+    return SourceRootFactory(self.get_options().lang_canonicalizations)
+
 
 class SourceRootTrie(object):
   """A trie for efficiently finding the source root for a path.
@@ -260,8 +306,8 @@ def new_child(self, key):
       self.children[key] = child
       return child
 
-  def __init__(self, lang_canonicalizations):
-    self._lang_canonicalizations = lang_canonicalizations
+  def __init__(self, source_root_factory):
+    self._source_root_factory = source_root_factory
     self._root = SourceRootTrie.Node()
 
   def add_pattern(self, pattern):
@@ -283,16 +329,6 @@ def _do_add_pattern(self, pattern, langs):
     node.langs = langs
     node.is_terminal = True
 
-  def _canonicalize_langs(self, langs):
-    ret = []
-    for lang in langs or []:
-      canonicalized = self._lang_canonicalizations.get(lang)
-      if canonicalized:
-        ret.extend(canonicalized)
-      else:
-        ret.append(lang)
-    return tuple(ret)
-
   def find(self, path):
     """Find the source root for the given path."""
     keys = ['^'] + path.split(os.path.sep)
@@ -310,6 +346,6 @@ def find(self, path):
           node = child
           j += 1
       if node.is_terminal:
-        return SourceRoot(os.path.join(*keys[1:j]), self._canonicalize_langs(langs))
+        return self._source_root_factory.create(os.path.join(*keys[1:j]), langs)
       # Otherwise, try the next value of i.
     return None

tests/python/pants_test/source/test_source_root.py

@@ -5,17 +5,17 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-from pants.source.source_root import SourceRoot, SourceRootConfig, SourceRootTrie
+from pants.source.source_root import SourceRoot, SourceRootConfig, SourceRootFactory, SourceRootTrie
 from pants_test.base_test import BaseTest
 from pants_test.subsystem.subsystem_util import create_subsystem
 
 
 class SourceRootTest(BaseTest):
   def test_source_root_trie(self):
-    trie = SourceRootTrie({
+    trie = SourceRootTrie(SourceRootFactory({
       'jvm': ('java', 'scala'),
       'py': ('python',)
-    })
+    }))
     self.assertIsNone(trie.find('src/java/org/pantsbuild/foo/Foo.java'))
 
     # Wildcard at the end.
@@ -81,7 +81,7 @@ def test_source_root_trie(self):
 
   def test_all_roots(self):
     self.create_dir('contrib/go/examples/3rdparty/go')
-    self.create_dir('contrib/go/examples/src/go')
+    self.create_dir('contrib/go/examples/src/go/src')
     self.create_dir('src/java')
     self.create_dir('src/python')
     self.create_dir('src/example/java')
@@ -91,15 +91,22 @@ def test_all_roots(self):
 
     options = {
       'source_root_patterns': ['src/*', 'src/example/*'],
-      # Test that our 'go_remote' hack works.
-      # TODO: This will be redundant once we have proper "3rdparty"/"remote" support.
-      'source_roots': { 'contrib/go/examples/3rdparty/go': ['go_remote'] }
+      'source_roots': {
+        # Fixed roots should trump patterns which would detect contrib/go/examples/src/go here.
+        'contrib/go/examples/src/go/src': ['go'],
+
+        # Test that our 'go_remote' hack works.
+        # TODO: This will be redundant once we have proper "3rdparty"/"remote" support.
+        'contrib/go/examples/3rdparty/go': ['go_remote'],
+
+        # Dir does not exist, should not be listed as a root.
+        'java': ['java']}
     }
     options.update(self.options[''])  # We need inherited values for pants_workdir etc.
 
     source_roots = create_subsystem(SourceRootConfig, **options).get_source_roots()
     self.assertEquals({SourceRoot('contrib/go/examples/3rdparty/go', ('go_remote',)),
-                       SourceRoot('contrib/go/examples/src/go', ('go',)),
+                       SourceRoot('contrib/go/examples/src/go/src', ('go',)),
                        SourceRoot('src/java', ('java',)),
                        SourceRoot('src/python', ('python',)),
                        SourceRoot('src/example/java', ('java',)),