diff --git a/contrib/go/src/python/pants/contrib/go/tasks/BUILD b/contrib/go/src/python/pants/contrib/go/tasks/BUILD index d4ad8af9763..91908879e3c 100644 --- a/contrib/go/src/python/pants/contrib/go/tasks/BUILD +++ b/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', diff --git a/contrib/go/src/python/pants/contrib/go/tasks/go_buildgen.py b/contrib/go/src/python/pants/contrib/go/tasks/go_buildgen.py index c8dd5575ed4..e3d7c8af045 100644 --- a/contrib/go/src/python/pants/contrib/go/tasks/go_buildgen.py +++ b/contrib/go/src/python/pants/contrib/go/tasks/go_buildgen.py @@ -11,9 +11,9 @@ 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 +from pants.util.memo import memoized from pants.contrib.go.subsystems.fetcher_factory import FetcherFactory from pants.contrib.go.targets.go_binary import GoBinary @@ -45,14 +45,23 @@ def __init__( local_root, fetcher_factory, generate_remotes=False, - remote_root=None, + remote_root_func=None, ): self._import_oracle = import_oracle self._build_graph = build_graph self._local_source_root = local_root self._fetcher_factory = fetcher_factory self._generate_remotes = generate_remotes - self._remote_source_root = remote_root + # We invoke self._remote_source_root_func lazily to create self._remote_source_root, + # then set it to None so we know that there's no need to reinvoke it. + self._remote_source_root_func = remote_root_func + self._remote_source_root = None + + def _get_remote_source_root(self): + if self._remote_source_root_func: + self._remote_source_root = self._remote_source_root_func() + self._remote_source_root_func = None + return self._remote_source_root def generate(self, local_go_targets): """Automatically generates a Go target graph for the given local go targets. @@ -65,7 +74,10 @@ def generate(self, local_go_targets): for local_go_target in local_go_targets: deps = self._list_deps(gopath, local_go_target.address) self._generate_missing(gopath, local_go_target.address, deps, visited) - return list(visited.items()) + # We don't want to trigger an exception if there's no remote root and we didn't need + # one, so we return self._remote_source_root directly, instead of calling + # self._get_remote_source_root(). + return list(visited.items()), self._remote_source_root def _generate_missing(self, gopath, local_address, import_listing, visited): target_type = GoBinary if import_listing.pkg_name == "main" else GoLibrary @@ -88,7 +100,9 @@ def _generate_missing(self, gopath, local_address, import_listing, visited): remote_root, import_path ) name = remote_pkg_path or os.path.basename(import_path) - address = Address(os.path.join(self._remote_source_root, remote_root), name) + address = Address( + os.path.join(self._get_remote_source_root(), remote_root), name + ) try: self._build_graph.inject_address_closure(address) except AddressLookupError: @@ -215,6 +229,15 @@ def register_options(cls, register): help="An optional extension for all materialized BUILD files (should include the .)", ) + register( + "--remote-root", + default=None, + metavar="", + 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 +386,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 +418,41 @@ 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)), - ) + @memoized + def infer_remote_root(): + inferrable_roots = ["3rdparty/go", "3rd_party/go", "thirdparty/go", "third_party/go"] + msg = ( + f"You can provide an explicit remote source root by setting remote_root in the " + f"[{self.options_scope}] section of your config file." + ) + rr = None + for path in inferrable_roots: + if os.path.isdir(path): + if rr: + raise self.InvalidRemoteRootsError( + f"Couldn't infer a single remote root. Found {rr} and {path}. {msg}" + ) + rr = path + if not rr: + raise self.InvalidRemoteRootsError( + f"Couldn't infer remote root. Found none of: {','.join(inferrable_roots)}. {msg}" ) - 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 + return rr - 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_func = lambda: self.get_options().remote_root or infer_remote_root() generator = GoTargetGenerator( self.import_oracle, @@ -456,11 +460,11 @@ def generate_targets(self, local_go_targets=None): local_root, self.get_fetcher_factory(), generate_remotes=self.get_options().remote, - remote_root=remote_root, + remote_root_func=remote_root_func, ) with self.context.new_workunit("go.buildgen", labels=[WorkUnitLabel.MULTITOOL]): try: - generated = generator.generate(local_go_targets) + generated, remote_root = generator.generate(local_go_targets) return self.GenerationResult( generated=generated, local_root=local_root, remote_root=remote_root ) diff --git a/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_buildgen.py b/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_buildgen.py index d13eb25e823..3e0067fedcf 100644 --- a/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_buildgen.py +++ b/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)