[recipe_engine/path] Make use of checkout based Paths a hard error.

This changes config_types.NamedBasePath to raise a ValueError if
anything, anywhere, attempts to 'resolve' (i.e. render to string)
a Path using a 'checkout' dynamic path.

All downstream repos tracked by the try builders on the recipes-py
repo ( and which have autorollers set up) should already be
compatible with this change.

Practically, this is a tests-only impact - attempts to use checkout
based Paths in production flows would end up with nonsense paths
such as "None/path/to/thing".

If any other recipe repos are impacted by this, the options for
remedy are as follows:

  1. (Preferred) Stop using checkout based paths - they are
     effectively a magical global variable and are more trouble than
     they are worth. Instead, just construct, and pass around, Path
     objects (which are always absolute paths). However, this
     suggestion is not always practical for established codebases,
     which leads to the second remedy.

  2. If dropping use of checkout based paths is too hard in the
     short term (which is likely), you can add something like the
     following to the top of your test recipe's RunSteps method:

        api.path['checkout'] = api.path['start_dir']

     This will cause all uses of e.g. `api.path['checkout']` to
     resolve to `api.path['start_dir']`, and you will no longer see
     ValueErrors (or invalid paths starting with 'None').

Reland: The original CL landed and exposed a bug in the build repo
where checkout-based paths were, in fact, being used in production
without assigning to api.path['checkout'] first. They were un-caught
because they were were added to the $PATH environment variable,
meaning that they were effectively no-ops because they rendered to
"None/something/something", and only became a visible error once
this CL landed. We're adding tests and fixing the build code prior
to relanding this CL.

R=bpastene, bryner, sshrimp, gbeaty

Bug: 329113288
Change-Id: Iff86f758317c2816522a4ef6c82e093b94377600
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/recipes-py/+/5388750
Reviewed-by: Brian Ryner <bryner@google.com>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
diff --git a/README.recipes.md b/README.recipes.md
index f032d8a..00aa6aa 100644
--- a/README.recipes.md
+++ b/README.recipes.md
@@ -2827,13 +2827,13 @@
 `depot_tools/infra_paths` module). Refer to those modules for additional
 documentation.
 
-#### **class [PathApi](/recipe_modules/path/api.py#237)([RecipeApi](/recipe_engine/recipe_api.py#470)):**
+#### **class [PathApi](/recipe_modules/path/api.py#235)([RecipeApi](/recipe_engine/recipe_api.py#470)):**
 
-&mdash; **def [\_\_getitem\_\_](/recipe_modules/path/api.py#530)(self, name: str):**
+&mdash; **def [\_\_getitem\_\_](/recipe_modules/path/api.py#531)(self, name: str):**
 
 Gets the base path named `name`. See module docstring for more info.
 
-&mdash; **def [\_\_setitem\_\_](/recipe_modules/path/api.py#482)(self, pathname: Literal[CheckoutPathName], path: config_types.Path):**
+&mdash; **def [\_\_setitem\_\_](/recipe_modules/path/api.py#483)(self, pathname: Literal[CheckoutPathName], path: config_types.Path):**
 
 Sets the checkout path.
 
@@ -2843,7 +2843,7 @@
 
 The only valid value of `pathname` is the literal string CheckoutPathName.
 
-&mdash; **def [abs\_to\_path](/recipe_modules/path/api.py#425)(self, abs_string_path: str):**
+&mdash; **def [abs\_to\_path](/recipe_modules/path/api.py#423)(self, abs_string_path: str):**
 
 Converts an absolute path string `abs_string_path` to a real Path
 object, using the most appropriate known base path.
@@ -2869,28 +2869,28 @@
 Raises an ValueError if the preconditions are not met, otherwise returns the
 Path object.
 
-&mdash; **def [abspath](/recipe_modules/path/api.py#552)(self, path: (config_types.Path | str)):**
+&mdash; **def [abspath](/recipe_modules/path/api.py#553)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.abspath.
 
-&mdash; **def [assert\_absolute](/recipe_modules/path/api.py#365)(self, path: (config_types.Path | str)):**
+&mdash; **def [assert\_absolute](/recipe_modules/path/api.py#363)(self, path: (config_types.Path | str)):**
 
 Raises AssertionError if the given path is not an absolute path.
 
 Args:
   * path - The path to check.
 
-&mdash; **def [basename](/recipe_modules/path/api.py#556)(self, path: (config_types.Path | str)):**
+&mdash; **def [basename](/recipe_modules/path/api.py#557)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.path.basename.
 
-&emsp; **@checkout_dir.setter**<br>&mdash; **def [checkout\_dir](/recipe_modules/path/api.py#500)(self, path: config_types.Path):**
+&emsp; **@checkout_dir.setter**<br>&mdash; **def [checkout\_dir](/recipe_modules/path/api.py#501)(self, path: config_types.Path):**
 
 Sets the global variable `api.path.checkout_dir` to the given path.
 
     
 
-&mdash; **def [dirname](/recipe_modules/path/api.py#560)(self, path: (config_types.Path | str)):**
+&mdash; **def [dirname](/recipe_modules/path/api.py#561)(self, path: (config_types.Path | str)):**
 
 For "foo/bar/baz", return "foo/bar".
 
@@ -2903,7 +2903,7 @@
 
 Returns dirname of path
 
-&mdash; **def [eq](/recipe_modules/path/api.py#726)(self, path1: config_types.Path, path2: config_types.Path):**
+&mdash; **def [eq](/recipe_modules/path/api.py#727)(self, path1: config_types.Path, path2: config_types.Path):**
 
 Check whether path1 points to the same path as path2.
 
@@ -2913,32 +2913,32 @@
 that problem by creating copies of the paths, and then separating them
 according to self.sep. The original paths are not modified.
 
-&mdash; **def [exists](/recipe_modules/path/api.py#664)(self, path):**
+&mdash; **def [exists](/recipe_modules/path/api.py#665)(self, path):**
 
 Equivalent to os.path.exists.
 
 The presence or absence of paths can be mocked during the execution of the
 recipe by using the mock_* methods.
 
-&mdash; **def [expanduser](/recipe_modules/path/api.py#655)(self, path):**
+&mdash; **def [expanduser](/recipe_modules/path/api.py#656)(self, path):**
 
 Do not use this, use `api.path['home']` instead.
 
 This ONLY handles `path` == "~", and returns `str(api.path['home'])`.
 
-&mdash; **def [get](/recipe_modules/path/api.py#518)(self, name: str, default: (config_types.Path | None)=None):**
+&mdash; **def [get](/recipe_modules/path/api.py#519)(self, name: str, default: (config_types.Path | None)=None):**
 
 Gets the base path named `name`. See module docstring for more info.
 
-&mdash; **def [get\_config\_defaults](/recipe_modules/path/api.py#249)(self):**
+&mdash; **def [get\_config\_defaults](/recipe_modules/path/api.py#247)(self):**
 
 Internal recipe implementation function.
 
-&mdash; **def [initialize](/recipe_modules/path/api.py#312)(self):**
+&mdash; **def [initialize](/recipe_modules/path/api.py#310)(self):**
 
 Internal recipe implementation function.
 
-&mdash; **def [is\_parent\_of](/recipe_modules/path/api.py#741)(self, parent: config_types.Path, child: config_types.Path):**
+&mdash; **def [is\_parent\_of](/recipe_modules/path/api.py#742)(self, parent: config_types.Path, child: config_types.Path):**
 
 Check whether child is contained within parent.
 
@@ -2949,21 +2949,21 @@
 the paths, and then separating them according to self.sep. The original
 paths are not modified.
 
-&mdash; **def [isdir](/recipe_modules/path/api.py#672)(self, path):**
+&mdash; **def [isdir](/recipe_modules/path/api.py#673)(self, path):**
 
 Equivalent to os.path.isdir.
 
 The presence or absence of paths can be mocked during the execution of the
 recipe by using the mock_* methods.
 
-&mdash; **def [isfile](/recipe_modules/path/api.py#680)(self, path):**
+&mdash; **def [isfile](/recipe_modules/path/api.py#681)(self, path):**
 
 Equivalent to os.path.isfile.
 
 The presence or absence of paths can be mocked during the execution of the
 recipe by using the mock_* methods.
 
-&mdash; **def [join](/recipe_modules/path/api.py#579)(self, path, \*paths):**
+&mdash; **def [join](/recipe_modules/path/api.py#580)(self, path, \*paths):**
 
 Equivalent to os.path.join.
 
@@ -2977,7 +2977,7 @@
 retrieved with api.path[something]), then you can convert from a string path
 back to a Path with the `abs_to_path` method.
 
-&mdash; **def [mkdtemp](/recipe_modules/path/api.py#374)(self, prefix: str=tempfile.template):**
+&mdash; **def [mkdtemp](/recipe_modules/path/api.py#372)(self, prefix: str=tempfile.template):**
 
 Makes a new temporary directory, returns Path to it.
 
@@ -2986,7 +2986,7 @@
 
 Returns a Path to the new directory.
 
-&mdash; **def [mkstemp](/recipe_modules/path/api.py#398)(self, prefix: str=tempfile.template):**
+&mdash; **def [mkstemp](/recipe_modules/path/api.py#396)(self, prefix: str=tempfile.template):**
 
 Makes a new temporary file, returns Path to it.
 
@@ -2996,23 +2996,23 @@
 Returns a Path to the new file. Unlike tempfile.mkstemp, the file's file
 descriptor is closed.
 
-&mdash; **def [mock\_add\_directory](/recipe_modules/path/api.py#698)(self, path: config_types.Path):**
+&mdash; **def [mock\_add\_directory](/recipe_modules/path/api.py#699)(self, path: config_types.Path):**
 
 For testing purposes, mark that file |path| exists.
 
-&mdash; **def [mock\_add\_file](/recipe_modules/path/api.py#694)(self, path: config_types.Path):**
+&mdash; **def [mock\_add\_file](/recipe_modules/path/api.py#695)(self, path: config_types.Path):**
 
 For testing purposes, mark that file |path| exists.
 
-&mdash; **def [mock\_add\_paths](/recipe_modules/path/api.py#688)(self, path: config_types.Path, kind: FileType=FileType.FILE):**
+&mdash; **def [mock\_add\_paths](/recipe_modules/path/api.py#689)(self, path: config_types.Path, kind: FileType=FileType.FILE):**
 
 For testing purposes, mark that |path| exists.
 
-&mdash; **def [mock\_copy\_paths](/recipe_modules/path/api.py#702)(self, source: config_types.Path, dest: config_types.Path):**
+&mdash; **def [mock\_copy\_paths](/recipe_modules/path/api.py#703)(self, source: config_types.Path, dest: config_types.Path):**
 
 For testing purposes, copy |source| to |dest|.
 
-&mdash; **def [mock\_remove\_paths](/recipe_modules/path/api.py#708)(self, path: config_types.Path, should_remove: Callable[([str], bool)]=(lambda p: True)):**
+&mdash; **def [mock\_remove\_paths](/recipe_modules/path/api.py#709)(self, path: config_types.Path, should_remove: Callable[([str], bool)]=(lambda p: True)):**
 
 For testing purposes, mark that |path| doesn't exist.
 
@@ -3021,38 +3021,38 @@
   should_remove: Called for every candidate path. Return True to remove this
     path.
 
-&mdash; **def [normpath](/recipe_modules/path/api.py#651)(self, path):**
+&mdash; **def [normpath](/recipe_modules/path/api.py#652)(self, path):**
 
 Equivalent to os.path.normpath.
 
-&emsp; **@property**<br>&mdash; **def [pardir](/recipe_modules/path/api.py#537)(self):**
+&emsp; **@property**<br>&mdash; **def [pardir](/recipe_modules/path/api.py#538)(self):**
 
 Equivalent to os.pardir.
 
-&emsp; **@property**<br>&mdash; **def [pathsep](/recipe_modules/path/api.py#547)(self):**
+&emsp; **@property**<br>&mdash; **def [pathsep](/recipe_modules/path/api.py#548)(self):**
 
 Equivalent to os.pathsep.
 
-&mdash; **def [realpath](/recipe_modules/path/api.py#639)(self, path: (config_types.Path | str)):**
+&mdash; **def [realpath](/recipe_modules/path/api.py#640)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.path.realpath.
 
-&mdash; **def [relpath](/recipe_modules/path/api.py#643)(self, path, start):**
+&mdash; **def [relpath](/recipe_modules/path/api.py#644)(self, path, start):**
 
 Roughly equivalent to os.path.relpath.
 
 Unlike os.path.relpath, `start` is _required_. If you want the 'current
 directory', use the `recipe_engine/context` module's `cwd` property.
 
-&emsp; **@property**<br>&mdash; **def [sep](/recipe_modules/path/api.py#542)(self):**
+&emsp; **@property**<br>&mdash; **def [sep](/recipe_modules/path/api.py#543)(self):**
 
 Equivalent to os.sep.
 
-&mdash; **def [separate](/recipe_modules/path/api.py#722)(self, path: config_types.Path):**
+&mdash; **def [separate](/recipe_modules/path/api.py#723)(self, path: config_types.Path):**
 
 Separate a path's pieces in-place with this platform's separator char.
 
-&mdash; **def [split](/recipe_modules/path/api.py#594)(self, path):**
+&mdash; **def [split](/recipe_modules/path/api.py#595)(self, path):**
 
 For "foo/bar/baz", return ("foo/bar", "baz").
 
@@ -3066,7 +3066,7 @@
 
 Returns (dirname(path), basename(path)).
 
-&mdash; **def [splitext](/recipe_modules/path/api.py#615)(self, path: (config_types.Path | str)):**
+&mdash; **def [splitext](/recipe_modules/path/api.py#616)(self, path: (config_types.Path | str)):**
 
 For "foo/bar.baz", return ("foo/bar", ".baz").
 
diff --git a/recipe_engine/config_types.py b/recipe_engine/config_types.py
index c67ad07..f350752 100644
--- a/recipe_engine/config_types.py
+++ b/recipe_engine/config_types.py
@@ -83,10 +83,9 @@
   def resolve(self, test_enabled: bool) -> str:
     if self.name == self._API.CheckoutPathName:
       checkout_dir = self._API.checkout_dir
-      # TODO: Enable this exception check.
-      # if checkout_dir is None:
-      #  raise ValueError(
-      #      f'Cannot resolve NamedBasePath({self.name!r}) - api.path.checkout_dir is unset.')
+      if checkout_dir is None:
+        raise ValueError(
+            f'Cannot resolve NamedBasePath({self.name!r}) - api.path.checkout_dir is unset.')
       return str(checkout_dir)
 
     if self.name in self._API.c.base_paths:
diff --git a/recipe_modules/path/api.py b/recipe_modules/path/api.py
index 629b0f0..717a837 100644
--- a/recipe_modules/path/api.py
+++ b/recipe_modules/path/api.py
@@ -37,10 +37,8 @@
 from __future__ import annotations
 
 import collections
-from collections.abc import Iterable
 import copy
 import enum
-import itertools
 import os
 import re
 import tempfile
@@ -459,7 +457,10 @@
         abs_string_path, self.sep)
     if path is None:
       # try base paths now
-      for path_name in itertools.chain((CheckoutPathName,), self.c.base_paths):
+      to_try = self.c.base_paths.keys()
+      if self.checkout_dir is not None:
+        to_try = [CheckoutPathName] + list(to_try)
+      for path_name in to_try:
         path = self[path_name]
         sPath = str(path)
         if abs_string_path.startswith(sPath):