Recognize multiple sentinel files for determining the build root (pan…

…tsbuild#8105)

## Problem
We cannot have implicit runtime dependencies on files in the buildroot - they must all be declared explicitly via `BUILD` entries. pantsbuild#8066 fixed several of these issues, but not all. 

However, we cannot explicitly depend on the file `./pants` because it breaks V1 test execution. When running tests with V1, the folder in `.pants.d` will strip the prefix so `src/python/pants` -> `pants`. You cannot both have a directory named `pants` and a file named `pants`, so this causes most V1 tests to fail.

## Solution
Use multiple sentinel files to establish the buildroot, such as `BUILD_ROOT`. This allows us to safely depend on a sentinel file with very few code changes.

BUILD

@@ -1,6 +1,13 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+# We use this to establish the build root, rather than `./pants`, because we cannot safely use the
+# latter as the sentinel filename per https://github.com/pantsbuild/pants/pull/8105.
+files(
+  name = 'build_root',
+  source = "BUILD_ROOT",
+)
+
 files(
   name = 'build_tools',
   source = 'BUILD.tools',
@@ -10,3 +17,8 @@ files(
   name = '3rdparty_build',
   source = '3rdparty/BUILD',
 )
+
+files(
+  name = 'pants_ini',
+  source = 'pants.ini'
+)

BUILD_ROOT

@@ -0,0 +1,2 @@
+# Used for our tests to find the build root, as we cannot safely use `./pants`
+# for the sentinel filename per https://github.com/pantsbuild/pants/pull/8105.

build-support/unit_test_remote_blacklist.txt

@@ -1,13 +1,2 @@
-tests/python/pants_test/backend/jvm/subsystems:shader
-tests/python/pants_test/backend/jvm/targets:jar_dependency
-tests/python/pants_test/backend/jvm/tasks/jvm_compile/java:java_compile_settings_partitioning
 tests/python/pants_test/backend/jvm/tasks/jvm_compile:jvm_compile
 tests/python/pants_test/backend/native/subsystems:subsystems
-tests/python/pants_test/base:build_root
-tests/python/pants_test/build_graph:build_file_aliases
-tests/python/pants_test/init:init
-tests/python/pants_test/invalidation:build_invalidator
-tests/python/pants_test/ivy:bootstrapper
-tests/python/pants_test/ivy:ivy_subsystem
-tests/python/pants_test/java/distribution:distribution
-tests/python/pants_test/option:testing

src/docs/install.md

@@ -18,7 +18,7 @@ Recommended Installation
 ------------------------
 
 To set up Pants in your repo, we recommend installing our self-contained `pants` bash script
-in the root (i.e. the "buildroot") of your repo:
+in the root (i.e. the "build root") of your repo:
 
     :::bash
     curl -L -O https://pantsbuild.github.io/setup/pants && chmod +x pants
@@ -51,16 +51,12 @@ The ./pants Runner Script
 -------------------------
 
 We highly recommend invoking pants via a checked-in runner script named `pants` in the
-root of your workspace, as demonstrated above.  Pants uses the presence of such a file, in the
-current working directory or in any of its ancestors, to detect the buildroot, e.g., when
+root of your workspace, as demonstrated above. Pants uses the presence of such a file, in the
+current working directory or in any of its ancestors, to detect the build root, e.g., when
 invoked in a subdirectory.
 
-If, for whatever reason, you don't want to run pants that way, you can also just check in an
-empty file named `pants` to act as a sentinel for the buildroot.
-
-Note that you can create whatever symlinks or extra wrapper scripts you like.  There's no absolute
-requirement that pants be invoked directly via `./pants`.  All pants cares about is the existence
-of a file named `pants` in the buildroot, and that file might as well be the runner script!
+If, for whatever reason, you don't want to run Pants that way, you can also just check in an
+empty file named `BUILD_ROOT` to act as the sentinel for determining your project's build root.
 
 PEX-based Installation
 ----------------------

src/python/pants/backend/jvm/subsystems/zinc.py

@@ -3,6 +3,7 @@
 
 import os
 from hashlib import sha1
+from pathlib import Path
 from threading import Lock
 
 from pants.backend.jvm.subsystems.dependency_context import DependencyContext
@@ -21,7 +22,7 @@
 from pants.engine.isolated_process import ExecuteProcessRequest
 from pants.java.distribution.distribution import Distribution
 from pants.java.jar.jar_dependency import JarDependency
-from pants.util.dirutil import fast_relpath, safe_delete, safe_mkdir
+from pants.util.dirutil import fast_relpath, safe_delete, safe_mkdir, safe_mkdir_for
 from pants.util.fileutil import safe_hardlink_or_copy
 from pants.util.memo import memoized_method, memoized_property
 
@@ -212,37 +213,30 @@ def native_image(self, context):
     return self._zinc_factory.hackily_snapshot(context)
 
   @memoized_property
-  def dist(self):
-    """Return the `Distribution` selected for Zinc based on execution strategy.
-
-    :rtype: pants.java.distribution.distribution.Distribution
-    """
+  def dist(self) -> Distribution:
+    """Return the `Distribution` selected for Zinc based on execution strategy."""
     underlying_dist = self.underlying_dist
-    if self._execution_strategy != NailgunTaskBase.HERMETIC:
-      # symlink .pants.d/.jdk -> /some/java/home/
-      jdk_home_symlink = os.path.relpath(
-        os.path.join(self._zinc_factory.get_options().pants_workdir, '.jdk'),
-        get_buildroot())
-
-      # Since this code can be run in multi-threading mode due to multiple
-      # zinc workers, we need to make sure the file operations below are atomic.
-      with self._lock:
-        # Create the symlink if it does not exist, or points to a file that doesn't exist,
-        # (e.g., a JDK that is no longer present), or points to the wrong JDK.
-        if (not os.path.exists(jdk_home_symlink) or
-            os.readlink(jdk_home_symlink) != underlying_dist.home):
-          safe_delete(jdk_home_symlink)  # Safe-delete, in case it's a broken symlink.
-          os.symlink(underlying_dist.home, jdk_home_symlink)
-
-      return Distribution(home_path=jdk_home_symlink)
-    else:
+    if self._execution_strategy == NailgunTaskBase.HERMETIC:
       return underlying_dist
+    # symlink .pants.d/.jdk -> /some/java/home/
+    jdk_home_symlink = (
+      Path(self._zinc_factory.get_options().pants_workdir) / '.jdk'
+    ).relative_to(get_buildroot())
+
+    # Since this code can be run in multi-threading mode due to multiple
+    # zinc workers, we need to make sure the file operations below are atomic.
+    with self._lock:
+      # Create the symlink if it does not exist, or points to a file that doesn't exist,
+      # (e.g., a JDK that is no longer present), or points to the wrong JDK.
+      if not jdk_home_symlink.exists() or jdk_home_symlink.resolve() != Path(underlying_dist.home):
+        safe_delete(str(jdk_home_symlink))  # Safe-delete, in case it's a broken symlink.
+        safe_mkdir_for(jdk_home_symlink)
+        jdk_home_symlink.symlink_to(underlying_dist.home)
+
+    return Distribution(home_path=jdk_home_symlink)
 
   @property
-  def underlying_dist(self):
-    """
-    :rtype: pants.java.distribution.distribution.Distribution
-    """
+  def underlying_dist(self) -> Distribution:
     return self._zinc_factory.dist
 
   @memoized_property

src/python/pants/base/build_environment.py

@@ -23,8 +23,8 @@ def pants_release():
           .format(version=pants_version()))
 
 
-def get_buildroot():
-  """Returns the pants build root, calculating it if needed.
+def get_buildroot() -> str:
+  """Returns the Pants build root, calculating it if needed.
 
   :API: public
   """
@@ -49,7 +49,7 @@ def get_pants_configdir():
   return os.path.expanduser(os.path.join(config_home, 'pants'))
 
 
-def get_default_pants_config_file():
+def get_default_pants_config_file() -> str:
   """Return the default location of the pants config file."""
   return os.path.join(get_buildroot(), 'pants.ini')
 

src/python/pants/base/build_root.py

@@ -3,6 +3,7 @@
 
 import os
 from contextlib import contextmanager
+from pathlib import Path
 
 from pants.util.meta import Singleton
 
@@ -11,33 +12,30 @@
 class BuildRoot(Singleton):
   """Represents the global workspace build root.
 
-  By default a pants workspace is defined by a root directory where a file called 'pants' -
-  typically the pants runner script - lives.  This path can also be manipulated through
-  this interface for re-location of the build root in tests.
-
-  TODO: If this ever causes a problem (because some subdir that people run pants in
-        legitimately contains a file called 'pants') then we can add a second check for
-        an explicit sentinel file, like 'BUILDROOT'.
+  By default a Pants workspace is defined by a root directory where one of multiple sentinel files
+  reside, such as `pants` or `BUILD_ROOT`. This path can also be manipulated through this interface
+  for re-location of the build root in tests.
   """
 
+  sentinel_files = ["pants", "BUILDROOT", "BUILD_ROOT"]
+
   class NotFoundError(Exception):
     """Raised when unable to find the current workspace build root."""
 
-  @classmethod
-  def find_buildroot(cls):
-    buildroot = os.path.abspath(os.getcwd())
-    while not os.path.isfile(os.path.join(buildroot, 'pants')):
-      parent = os.path.dirname(buildroot)
-      if buildroot != parent:
-        buildroot = parent
+  def find_buildroot(self) -> str:
+    buildroot = Path.cwd().resolve()
+    while not any((buildroot / sentinel).is_file() for sentinel in self.sentinel_files):
+      if buildroot != buildroot.parent:
+        buildroot = buildroot.parent
       else:
-        raise cls.NotFoundError('No buildroot detected. Pants detects the buildroot by looking '
-                                'for a file named pants in the cwd and its ancestors.  Typically '
-                                'this is the runner script that executes pants.  If you have no '
-                                'such script you can create an empty file in your buildroot.')
-    return buildroot
+        raise self.NotFoundError(
+          'No build root detected. Pants detects the build root by looking for at least one file '
+          f'from {self.sentinel_files} in the cwd and its ancestors. If you have none of these '
+          f'files, you can create an empty file in your build root.'
+        )
+    return str(buildroot)
 
-  def __init__(self):
+  def __init__(self) -> None:
     self._root_dir = None
 
   @property
@@ -57,15 +55,15 @@ def path(self, root_dir):
     """Manually establishes the build root for the current workspace."""
     path = os.path.realpath(root_dir)
     if not os.path.exists(path):
-      raise ValueError('Build root does not exist: {}'.format(root_dir))
+      raise ValueError(f'Build root does not exist: {root_dir}')
     self._root_dir = path
 
   def reset(self):
     """Clears the last calculated build root for the current workspace."""
     self._root_dir = None
 
   def __str__(self):
-    return 'BuildRoot({})'.format(self._root_dir)
+    return f'BuildRoot({self._root_dir})'
 
   @contextmanager
   def temporary(self, path):

src/python/pants/option/config.py

@@ -51,7 +51,7 @@ def opener(file_content):
       with io.BytesIO(file_content.content) as fh:
         yield fh
 
-    return cls._meta_load(opener, file_contents, seed_values)
+    return cls._meta_load(opener, file_contents, seed_values=seed_values)
 
   @classmethod
   def load(cls, config_paths, seed_values=None):
@@ -72,16 +72,16 @@ def opener(f):
       with open(f, 'rb') as fh:
         yield fh
 
-    return cls._meta_load(opener, config_paths, seed_values)
+    return cls._meta_load(opener, config_paths, seed_values=seed_values)
 
   @classmethod
-  def _meta_load(cls, open_ctx, config_items, seed_values=None):
+  def _meta_load(cls, open_ctx, config_items, *, seed_values=None):
     if not config_items:
       return _EmptyConfig()
 
     single_file_configs = []
     for config_item in config_items:
-      parser = cls._create_parser(seed_values)
+      parser = cls._create_parser(seed_values=seed_values)
       with open_ctx(config_item) as ini:
         content = ini.read()
         content_digest = sha1(content).hexdigest()
@@ -92,7 +92,7 @@ def _meta_load(cls, open_ctx, config_items, seed_values=None):
     return _ChainedConfig(tuple(reversed(single_file_configs)))
 
   @classmethod
-  def _create_parser(cls, seed_values=None):
+  def _create_parser(cls, *, seed_values=None):
     """Creates a config parser that supports %([key-name])s value substitution.
 
     A handful of seed values will be set to act as if specified in the loaded config file's DEFAULT
@@ -103,7 +103,9 @@ def _create_parser(cls, seed_values=None):
                         pants_supportdir and pants_distdir.
     """
     seed_values = seed_values or {}
-    buildroot = seed_values.get('buildroot', get_buildroot())
+    buildroot = seed_values.get('buildroot')
+    if buildroot is None:
+      buildroot = get_buildroot()
 
     all_seed_values = {
       'buildroot': buildroot,

src/python/pants/option/global_options.py

@@ -117,7 +117,7 @@ def register_bootstrap_options(cls, register):
     buildroot = get_buildroot()
     default_distdir_name = 'dist'
     default_distdir = os.path.join(buildroot, default_distdir_name)
-    default_rel_distdir = '/{}/'.format(default_distdir_name)
+    default_rel_distdir = f'/{default_distdir_name}/'
 
     register('-l', '--level', choices=['trace', 'debug', 'info', 'warn'], default='info',
              recursive=True, help='Set the logging level.')

tests/python/pants_test/base/BUILD

@@ -15,6 +15,7 @@ python_tests(
   name = 'build_root',
   sources = ['test_build_root.py'],
   dependencies = [
+    '//:build_root',
     'src/python/pants/base:build_root',
     'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',

tests/python/pants_test/base/test_build_root.py

@@ -12,53 +12,54 @@
 class BuildRootTest(unittest.TestCase):
 
   def setUp(self):
-    self.original_root = BuildRoot().path
-    self.new_root = os.path.realpath(safe_mkdtemp())
-    BuildRoot().reset()
+    self.build_root = BuildRoot()
+    self.original_path = self.build_root.path
+    self.new_path = os.path.realpath(safe_mkdtemp())
+    self.build_root.reset()
 
   def tearDown(self):
-    BuildRoot().reset()
-    safe_rmtree(self.new_root)
+    self.build_root.reset()
+    safe_rmtree(self.new_path)
 
   def test_via_set(self):
-    BuildRoot().path = self.new_root
-    self.assertEqual(self.new_root, BuildRoot().path)
+    self.build_root.path = self.new_path
+    self.assertEqual(self.new_path, self.build_root.path)
 
   def test_reset(self):
-    BuildRoot().path = self.new_root
-    BuildRoot().reset()
-    self.assertEqual(self.original_root, BuildRoot().path)
+    self.build_root.path = self.new_path
+    self.build_root.reset()
+    self.assertEqual(self.original_path, self.build_root.path)
 
   def test_via_pants_runner(self):
     with temporary_dir() as root:
       root = os.path.realpath(root)
-      touch(os.path.join(root, 'pants'))
+      touch(os.path.join(root, "BUILD_ROOT"))
       with pushd(root):
-        self.assertEqual(root, BuildRoot().path)
+        self.assertEqual(root, self.build_root.path)
 
-      BuildRoot().reset()
+      self.build_root.reset()
       child = os.path.join(root, 'one', 'two')
       safe_mkdir(child)
       with pushd(child):
-        self.assertEqual(root, BuildRoot().path)
+        self.assertEqual(root, self.build_root.path)
 
   def test_temporary(self):
-    with BuildRoot().temporary(self.new_root):
-      self.assertEqual(self.new_root, BuildRoot().path)
-    self.assertEqual(self.original_root, BuildRoot().path)
+    with self.build_root.temporary(self.new_path):
+      self.assertEqual(self.new_path, self.build_root.path)
+    self.assertEqual(self.original_path, self.build_root.path)
 
   def test_singleton(self):
     self.assertEqual(BuildRoot().path, BuildRoot().path)
-    BuildRoot().path = self.new_root
+    BuildRoot().path = self.new_path
     self.assertEqual(BuildRoot().path, BuildRoot().path)
 
   def test_not_found(self):
     with temporary_dir() as root:
       root = os.path.realpath(root)
       with pushd(root):
-        self.assertRaises(BuildRoot.NotFoundError, lambda: BuildRoot().path)
+        self.assertRaises(BuildRoot.NotFoundError, lambda: self.build_root.path)
 
   def test_buildroot_override(self):
     with temporary_dir() as root:
       with environment_as(PANTS_BUILDROOT_OVERRIDE=root):
-        self.assertEqual(BuildRoot().path, root)
+        self.assertEqual(self.build_root.path, root)