Simplify Go buildgen's use of source roots.

[ci skip-rust-tests]
[ci skip-jvm-tests]

contrib/go/src/python/pants/contrib/go/tasks/BUILD

@@ -15,7 +15,6 @@ python_library(
     'src/python/pants/build_graph',
     'src/python/pants/option',
     'src/python/pants/process',
-    'src/python/pants/source',
     'src/python/pants/task',
     'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',

contrib/go/src/python/pants/contrib/go/tasks/go_buildgen.py

@@ -11,7 +11,6 @@
 from pants.base.workunit import WorkUnitLabel
 from pants.build_graph.address import Address
 from pants.build_graph.address_lookup_error import AddressLookupError
-from pants.source.source_root import SourceRootCategories
 from pants.util.contextutil import temporary_dir
 from pants.util.dirutil import safe_mkdir, safe_open
 
@@ -35,6 +34,9 @@ class WrongLocalSourceTargetTypeError(GenerationError):
         For example, a Go main package was defined as a GoLibrary instead of a GoBinary.
         """
 
+    class RemoteRequired(GenerationError):
+        """Indicates that the --remote-root was required but not specified."""
+
     class NewRemoteEncounteredButRemotesNotAllowedError(GenerationError):
         """Indicates a new remote library dependency was found but --remote was not enabled."""
 
@@ -88,6 +90,10 @@ def _generate_missing(self, gopath, local_address, import_listing, visited):
                             remote_root, import_path
                         )
                         name = remote_pkg_path or os.path.basename(import_path)
+                        if not self._remote_source_root:
+                            raise self.RemoteRequired(
+                                "You must provide the remote source root with --remote-root"
+                            )
                         address = Address(os.path.join(self._remote_source_root, remote_root), name)
                         try:
                             self._build_graph.inject_address_closure(address)
@@ -215,6 +221,15 @@ def register_options(cls, register):
             help="An optional extension for all materialized BUILD files (should include the .)",
         )
 
+        register(
+            "--remote-root",
+            default=None,
+            metavar="<path>",
+            advanced=True,
+            fingerprint=True,
+            help="Path to the remote golang source root, if any.",
+        )
+
     def execute(self):
         materialize = self.get_options().materialize
         if materialize:
@@ -363,15 +378,9 @@ def gather_go_buildfiles(rel_path):
             )
             raise self.FloatingRemoteError("Un-pinned (FLOATING) Go remote libraries detected.")
 
-    class NoLocalRootsError(TaskError):
-        """Indicates the Go local source owning targets' source roots are invalid."""
-
     class InvalidLocalRootsError(TaskError):
         """Indicates the Go local source owning targets' source roots are invalid."""
 
-    class UnrootedLocalSourceError(TaskError):
-        """Indicates there are Go local source owning targets that fall outside the source root."""
-
     class InvalidRemoteRootsError(TaskError):
         """Indicates the Go remote library source roots are invalid."""
 
@@ -401,54 +410,19 @@ def generate_targets(self, local_go_targets=None):
         # The GOPATH's 1st element is read-write, the rest are read-only; ie: their sources build to
         # the 1st element's pkg/ and bin/ dirs.
 
-        go_roots_by_category = defaultdict(list)
-        # TODO: Add "find source roots for lang" functionality to SourceRoots and use that instead.
-        for sr in self.context.source_roots.all_roots():
-            if "go" in sr.langs:
-                go_roots_by_category[sr.category].append(sr.path)
-
-        if go_roots_by_category[SourceRootCategories.TEST]:
-            raise self.InvalidLocalRootsError("Go buildgen does not support test source roots.")
-        if go_roots_by_category[SourceRootCategories.UNKNOWN]:
-            raise self.InvalidLocalRootsError(
-                "Go buildgen does not support source roots of " "unknown category."
-            )
+        if not local_go_targets:
+            local_go_targets = self.context.scan().targets(self.is_local_src)
+            if not local_go_targets:
+                return None
 
-        local_roots = go_roots_by_category[SourceRootCategories.SOURCE]
-        if not local_roots:
-            raise self.NoLocalRootsError(
-                "Can only BUILD gen if a Go local sources source root is " "defined."
-            )
+        local_roots = {t.target_base for t in local_go_targets}
         if len(local_roots) > 1:
             raise self.InvalidLocalRootsError(
-                "Can only BUILD gen for a single Go local sources source "
-                "root, found:\n\t{}".format("\n\t".join(sorted(local_roots)))
+                "BUILD gen requires a single local Go source root, but found Go targets under "
+                f"multiple build roots: {', '.join(sorted(local_roots))}"
             )
         local_root = local_roots.pop()
-
-        if local_go_targets:
-            unrooted_locals = {t for t in local_go_targets if t.target_base != local_root}
-            if unrooted_locals:
-                raise self.UnrootedLocalSourceError(
-                    "Cannot BUILD gen until the following targets are "
-                    "relocated to the source root at {}:\n\t{}".format(
-                        local_root,
-                        "\n\t".join(sorted(t.address.reference() for t in unrooted_locals)),
-                    )
-                )
-        else:
-            root = os.path.join(get_buildroot(), local_root)
-            local_go_targets = self.context.scan(root=root).targets(self.is_local_src)
-            if not local_go_targets:
-                return None
-
-        remote_roots = go_roots_by_category[SourceRootCategories.THIRDPARTY]
-        if len(remote_roots) > 1:
-            raise self.InvalidRemoteRootsError(
-                "Can only BUILD gen for a single Go remote library source "
-                "root, found:\n\t{}".format("\n\t".join(sorted(remote_roots)))
-            )
-        remote_root = remote_roots.pop() if remote_roots else None
+        remote_root = self.get_options().remote_root
 
         generator = GoTargetGenerator(
             self.import_oracle,

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_buildgen.py

@@ -57,36 +57,19 @@ def test_noop_no_applicable_targets(self):
         task.execute()
         self.assertEqual(expected, context.targets())
 
-    def test_no_local_roots_failure(self):
-        context = self.context(target_roots=[self.make_target("src/go/src/fred", GoBinary)])
-        task = self.create_task(context)
-        with self.assertRaises(task.NoLocalRootsError):
-            task.execute()
-
     def test_multiple_local_roots_failure(self):
         self.create_dir("src/go/src")
         self.create_dir("src/main/go/src")
-        context = self.context(target_roots=[self.make_target("src/go/src/fred", GoBinary)])
+        context = self.context(
+            target_roots=[
+                self.make_target("src/go/src/fred", GoBinary),
+                self.make_target("src/main/go/src/barney", GoBinary),
+            ]
+        )
         task = self.create_task(context)
         with self.assertRaises(task.InvalidLocalRootsError):
             task.execute()
 
-    def test_unrooted_failure(self):
-        self.create_dir("src/go/src")
-        context = self.context(target_roots=[self.make_target("src2/go/src/fred", GoBinary)])
-        task = self.create_task(context)
-        with self.assertRaises(task.UnrootedLocalSourceError):
-            task.execute()
-
-    def test_multiple_remote_roots_failure(self):
-        self.create_dir("3rdparty/go")
-        self.create_dir("src/go/src/fred")
-        self.create_dir("other/3rdparty/go")
-        context = self.context(target_roots=[self.make_target("src/go/src/fred", GoLibrary)])
-        task = self.create_task(context)
-        with self.assertRaises(task.InvalidRemoteRootsError):
-            task.execute()
-
     def test_existing_targets_wrong_type(self):
         self.create_file(
             relpath="src/go/src/fred/foo.go",
@@ -285,6 +268,7 @@ def stitch_deps_remote(self, remote=True, materialize=False, fail_floating=False
             fred = self.make_target("src/go/src/fred", GoBinary)
             target_roots = [fred]
 
+        self.set_options(remote_root="3rdparty/go")
         context = self.context(target_roots=target_roots)
         pre_execute_files = self.buildroot_files()
         task = self.create_task(context)
@@ -350,42 +334,6 @@ def test_fail_floating(self):
         with self.assertRaises(GoBuildgen.FloatingRemoteError):
             self.stitch_deps_remote(remote=True, materialize=True, fail_floating=True)
 
-    def test_issues_2395(self):
-        # Previously, when a remote was indirectly discovered via a scan of locals (no target roots
-        # presented on the CLI), the remote would be queried for from the build graph under the
-        # erroneous assumption it had been injected.  This would result in a graph miss (BUILD file was
-        # there on disk, but never loaded via injection) and lead to creation of a new synthetic remote
-        # target with no rev.  The end result was lossy go remote library rev values when using the
-        # newer, encouraged, target-less invocation of GoBuildgen.
-
-        self.set_options(remote=True, materialize=True, fail_floating=True)
-        self.add_to_build_file(
-            relpath="3rdparty/go/pantsbuild.org/fake", target='go_remote_library(rev="v4.5.6")'
-        )
-
-        self.create_file(
-            relpath="src/go/src/jane/bar.go",
-            contents=dedent(
-                """
-                package jane
-        
-                import "pantsbuild.org/fake"
-        
-                var PublicConstant = fake.DoesNotExistButWeShouldNotCareWhenCheckingDepsAndNotInstalling
-                """
-            ),
-        )
-        self.add_to_build_file(relpath="src/go/src/jane", target="go_library()")
-
-        context = self.context(target_roots=[])
-        pre_execute_files = self.buildroot_files()
-        task = self.create_task(context)
-        task.execute()
-
-        self.build_graph.reset()  # Force targets to be loaded off disk
-        self.assertEqual("v4.5.6", self.target("3rdparty/go/pantsbuild.org/fake").rev)
-        self.assertEqual(pre_execute_files, self.buildroot_files())
-
     def test_issues_2616(self):
         self.set_options(remote=False)