[path] Handle api.path.checkout_dir better

Previously, using api.path['checkout'] would work, but
api.path.checkout_dir would not. Move the extra handling from Path.get()
(which Path.__getitem__() used) into the checkout_dir property, so
updated uses of checkout directories will still work.

Also, remove checkout_dir as a base in api.path.abs_to_path().
checkout_dir should be within one of the other bases, and removing it
from abs_to_path removes the need to handle uninitialized checkout
bases in abs_to_path.

Bug: 329113288
Change-Id: I1f1e6619ae2add0cd73fbc98eabf747f513b41b5
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/recipes-py/+/5456072
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Auto-Submit: Rob Mohr <mohrr@google.com>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
diff --git a/README.recipes.md b/README.recipes.md
index 5daebc3..eb2c8f7 100644
--- a/README.recipes.md
+++ b/README.recipes.md
@@ -2873,7 +2873,7 @@
 
 #### **class [PathApi](/recipe_modules/path/api.py#328)([RecipeApi](/recipe_engine/recipe_api.py#471)):**
 
-&mdash; **def [\_\_contains\_\_](/recipe_modules/path/api.py#588)(self, pathname: NamedBasePathsType):**
+&mdash; **def [\_\_contains\_\_](/recipe_modules/path/api.py#586)(self, pathname: NamedBasePathsType):**
 
 This method is DEPRECATED.
 
@@ -2888,7 +2888,7 @@
 a very complicated 'config' system. All of that has been removed, but this
 method remains for now.
 
-&mdash; **def [\_\_getitem\_\_](/recipe_modules/path/api.py#687)(self, name: NamedBasePathsType):**
+&mdash; **def [\_\_getitem\_\_](/recipe_modules/path/api.py#689)(self, name: NamedBasePathsType):**
 
 Gets the base path named `name`. See module docstring for more info.
 
@@ -2903,7 +2903,7 @@
   pass the Paths around instead of using this global variable).
 ***
 
-&mdash; **def [\_\_setitem\_\_](/recipe_modules/path/api.py#606)(self, pathname: CheckoutPathNameType, path: config_types.Path):**
+&mdash; **def [\_\_setitem\_\_](/recipe_modules/path/api.py#604)(self, pathname: CheckoutPathNameType, path: config_types.Path):**
 
 Sets the checkout path.
 
@@ -2926,7 +2926,6 @@
   * module resource paths
   * recipe resource paths
   * repo paths
-  * checkout_dir
   * home_dir
   * start_dir
   * tmp_base_dir
@@ -2942,7 +2941,7 @@
 Raises an ValueError if the preconditions are not met, otherwise returns the
 Path object.
 
-&mdash; **def [abspath](/recipe_modules/path/api.py#797)(self, path: (config_types.Path | str)):**
+&mdash; **def [abspath](/recipe_modules/path/api.py#799)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.abspath.
 
@@ -2953,11 +2952,11 @@
 Args:
   * path - The path to check.
 
-&mdash; **def [basename](/recipe_modules/path/api.py#801)(self, path: (config_types.Path | str)):**
+&mdash; **def [basename](/recipe_modules/path/api.py#803)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.path.basename.
 
-&emsp; **@property**<br>&mdash; **def [cache\_dir](/recipe_modules/path/api.py#731)(self):**
+&emsp; **@property**<br>&mdash; **def [cache\_dir](/recipe_modules/path/api.py#733)(self):**
 
 This directory is provided by whatever's running the recipe.
 
@@ -2980,7 +2979,7 @@
 Note that directories created under here /may/ be evicted in between runs of
 the recipe (i.e. to relieve disk pressure).
 
-&mdash; **def [cast\_to\_path](/recipe_modules/path/api.py#765)(self, strpath: str):**
+&mdash; **def [cast\_to\_path](/recipe_modules/path/api.py#767)(self, strpath: str):**
 
 This returns a Path for strpath which can be used anywhere a Path is
 required.
@@ -2993,20 +2992,20 @@
 cache_dir), the returned Path will be based on that known path. This is
 important for test compatibility.
 
-&emsp; **@checkout_dir.setter**<br>&mdash; **def [checkout\_dir](/recipe_modules/path/api.py#626)(self, path: config_types.Path):**
+&emsp; **@checkout_dir.setter**<br>&mdash; **def [checkout\_dir](/recipe_modules/path/api.py#628)(self, path: config_types.Path):**
 
 Sets the global variable `api.path.checkout_dir` to the given path.
 
     
 
-&emsp; **@property**<br>&mdash; **def [cleanup\_dir](/recipe_modules/path/api.py#756)(self):**
+&emsp; **@property**<br>&mdash; **def [cleanup\_dir](/recipe_modules/path/api.py#758)(self):**
 
 This directory is guaranteed to be cleaned up (eventually) after the
 execution of this recipe.
 
 This directory is guaranteed to be empty when the recipe starts.
 
-&mdash; **def [dirname](/recipe_modules/path/api.py#805)(self, path: (config_types.Path | str)):**
+&mdash; **def [dirname](/recipe_modules/path/api.py#807)(self, path: (config_types.Path | str)):**
 
 For "foo/bar/baz", return "foo/bar".
 
@@ -3019,7 +3018,7 @@
 
 Returns dirname of path
 
-&mdash; **def [eq](/recipe_modules/path/api.py#969)(self, path1: config_types.Path, path2: config_types.Path):**
+&mdash; **def [eq](/recipe_modules/path/api.py#971)(self, path1: config_types.Path, path2: config_types.Path):**
 
 Check whether path1 points to the same path as path2.
 
@@ -3027,20 +3026,20 @@
 **DEPRECATED**: Just directly compare path1 and path2 with `==`.
 ***
 
-&mdash; **def [exists](/recipe_modules/path/api.py#909)(self, path):**
+&mdash; **def [exists](/recipe_modules/path/api.py#911)(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#900)(self, path):**
+&mdash; **def [expanduser](/recipe_modules/path/api.py#902)(self, path):**
 
 Do not use this, use `api.path.home_dir` instead.
 
 This ONLY handles `path` == "~", and returns `str(api.path.home_dir)`.
 
-&mdash; **def [get](/recipe_modules/path/api.py#655)(self, name: NamedBasePathsType):**
+&mdash; **def [get](/recipe_modules/path/api.py#657)(self, name: NamedBasePathsType, \*, skip_deprecation=False):**
 
 Gets the base path named `name`. See module docstring for more info.
 
@@ -3055,7 +3054,7 @@
   pass the Paths around instead of using this global variable).
 ***
 
-&emsp; **@property**<br>&mdash; **def [home\_dir](/recipe_modules/path/api.py#713)(self):**
+&emsp; **@property**<br>&mdash; **def [home\_dir](/recipe_modules/path/api.py#715)(self):**
 
 This is the path to the current $HOME directory.
 
@@ -3067,7 +3066,7 @@
 This is called by the recipe engine immediately after __init__(), but
 with `self._paths_client` initialized.
 
-&mdash; **def [is\_parent\_of](/recipe_modules/path/api.py#976)(self, parent: config_types.Path, child: config_types.Path):**
+&mdash; **def [is\_parent\_of](/recipe_modules/path/api.py#978)(self, parent: config_types.Path, child: config_types.Path):**
 
 Check whether child is contained within parent.
 
@@ -3075,21 +3074,21 @@
 **DEPRECATED**: Just use `parent.is_parent_of(child)`.
 ***
 
-&mdash; **def [isdir](/recipe_modules/path/api.py#917)(self, path):**
+&mdash; **def [isdir](/recipe_modules/path/api.py#919)(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#925)(self, path):**
+&mdash; **def [isfile](/recipe_modules/path/api.py#927)(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#824)(self, path, \*paths):**
+&mdash; **def [join](/recipe_modules/path/api.py#826)(self, path, \*paths):**
 
 Equivalent to os.path.join.
 
@@ -3127,23 +3126,23 @@
 either a resource script of your recipe module or recipe.
 ***
 
-&mdash; **def [mock\_add\_directory](/recipe_modules/path/api.py#944)(self, path: config_types.Path):**
+&mdash; **def [mock\_add\_directory](/recipe_modules/path/api.py#946)(self, path: config_types.Path):**
 
 For testing purposes, mark that file |path| exists.
 
-&mdash; **def [mock\_add\_file](/recipe_modules/path/api.py#940)(self, path: config_types.Path):**
+&mdash; **def [mock\_add\_file](/recipe_modules/path/api.py#942)(self, path: config_types.Path):**
 
 For testing purposes, mark that file |path| exists.
 
-&mdash; **def [mock\_add\_paths](/recipe_modules/path/api.py#933)(self, path: config_types.Path, kind: FileType=FileType.FILE):**
+&mdash; **def [mock\_add\_paths](/recipe_modules/path/api.py#935)(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#948)(self, source: config_types.Path, dest: config_types.Path):**
+&mdash; **def [mock\_copy\_paths](/recipe_modules/path/api.py#950)(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#955)(self, path: config_types.Path, should_remove: Callable[([str], bool)]=(lambda p: True)):**
+&mdash; **def [mock\_remove\_paths](/recipe_modules/path/api.py#957)(self, path: config_types.Path, should_remove: Callable[([str], bool)]=(lambda p: True)):**
 
 For testing purposes, mark that |path| doesn't exist.
 
@@ -3152,34 +3151,34 @@
   should_remove: Called for every candidate path. Return True to remove this
     path.
 
-&mdash; **def [normpath](/recipe_modules/path/api.py#896)(self, path):**
+&mdash; **def [normpath](/recipe_modules/path/api.py#898)(self, path):**
 
 Equivalent to os.path.normpath.
 
-&emsp; **@property**<br>&mdash; **def [pardir](/recipe_modules/path/api.py#782)(self):**
+&emsp; **@property**<br>&mdash; **def [pardir](/recipe_modules/path/api.py#784)(self):**
 
 Equivalent to os.pardir.
 
-&emsp; **@property**<br>&mdash; **def [pathsep](/recipe_modules/path/api.py#792)(self):**
+&emsp; **@property**<br>&mdash; **def [pathsep](/recipe_modules/path/api.py#794)(self):**
 
 Equivalent to os.pathsep.
 
-&mdash; **def [realpath](/recipe_modules/path/api.py#884)(self, path: (config_types.Path | str)):**
+&mdash; **def [realpath](/recipe_modules/path/api.py#886)(self, path: (config_types.Path | str)):**
 
 Equivalent to os.path.realpath.
 
-&mdash; **def [relpath](/recipe_modules/path/api.py#888)(self, path, start):**
+&mdash; **def [relpath](/recipe_modules/path/api.py#890)(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#787)(self):**
+&emsp; **@property**<br>&mdash; **def [sep](/recipe_modules/path/api.py#789)(self):**
 
 Equivalent to os.sep.
 
-&mdash; **def [split](/recipe_modules/path/api.py#839)(self, path):**
+&mdash; **def [split](/recipe_modules/path/api.py#841)(self, path):**
 
 For "foo/bar/baz", return ("foo/bar", "baz").
 
@@ -3193,7 +3192,7 @@
 
 Returns (dirname(path), basename(path)).
 
-&mdash; **def [splitext](/recipe_modules/path/api.py#860)(self, path: (config_types.Path | str)):**
+&mdash; **def [splitext](/recipe_modules/path/api.py#862)(self, path: (config_types.Path | str)):**
 
 For "foo/bar.baz", return ("foo/bar", ".baz").
 
@@ -3208,7 +3207,7 @@
 Returns:
   (name, extension_including_dot).
 
-&emsp; **@property**<br>&mdash; **def [start\_dir](/recipe_modules/path/api.py#702)(self):**
+&emsp; **@property**<br>&mdash; **def [start\_dir](/recipe_modules/path/api.py#704)(self):**
 
 This is the directory that the recipe started in. it's similar to `cwd`,
 except that it's constant for the duration of the entire program.
@@ -3217,7 +3216,7 @@
 See the 'recipe_engine/context' module which allows modifying the cwd safely
 via a context manager.
 
-&emsp; **@property**<br>&mdash; **def [tmp\_base\_dir](/recipe_modules/path/api.py#722)(self):**
+&emsp; **@property**<br>&mdash; **def [tmp\_base\_dir](/recipe_modules/path/api.py#724)(self):**
 
 This directory is the system-configured temp dir.
 
@@ -5741,7 +5740,7 @@
 [DEPS](/recipe_modules/path/examples/full.py#7): [json](#recipe_modules-json), [path](#recipe_modules-path), [platform](#recipe_modules-platform), [properties](#recipe_modules-properties), [step](#recipe_modules-step)
 
 
-&emsp; **@recipe_api.ignore_warnings('recipe_engine/CHECKOUT_DIR_DEPRECATED')**<br>&mdash; **def [RunSteps](/recipe_modules/path/examples/full.py#18)(api):**
+&emsp; **@recipe_api.ignore_warnings('recipe_engine/CHECKOUT_DIR_DEPRECATED', 'recipe_engine/PATH_GETITEM_DEPRECATED')**<br>&mdash; **def [RunSteps](/recipe_modules/path/examples/full.py#18)(api):**
 ### *recipes* / [path:tests/cast\_to\_path](/recipe_modules/path/tests/cast_to_path.py)
 
 [DEPS](/recipe_modules/path/tests/cast_to_path.py#8): [path](#recipe_modules-path), [platform](#recipe_modules-platform)
diff --git a/recipe_modules/path/api.py b/recipe_modules/path/api.py
index 1846437..3638683 100644
--- a/recipe_modules/path/api.py
+++ b/recipe_modules/path/api.py
@@ -534,7 +534,6 @@
       * module resource paths
       * recipe resource paths
       * repo paths
-      * checkout_dir
       * home_dir
       * start_dir
       * tmp_base_dir
@@ -563,7 +562,6 @@
     if path is None:
       to_try = [
           self.cache_dir,
-          self.checkout_dir,
           self.cleanup_dir,
           self.home_dir,
           self.start_dir,
@@ -600,7 +598,7 @@
     method remains for now.
     """
     if pathname == self.CheckoutPathName:
-      return bool(self.checkout_dir)
+      return bool(self._checkout_dir)
     return pathname in self.NamedBasePaths
 
   def __setitem__(self, pathname: CheckoutPathNameType,
@@ -621,7 +619,11 @@
   @property
   def checkout_dir(self) -> config_types.Path|None:
     """Returns the Path which was assigned to this checkout_dir property."""
-    return self._checkout_dir
+    if cdir := self._checkout_dir:
+      # If the checkout_dir is already set, just return it directly.
+      return cdir
+    # In this case, the checkout_dir is not yet set, but it could be later.
+    return config_types.Path(config_types.CheckoutBasePath())
 
   @checkout_dir.setter
   def checkout_dir(self, path: config_types.Path) -> None:
@@ -652,7 +654,8 @@
       assert isinstance(self._path_mod, fake_path)
       self._path_mod._mock_path_exists.mark_checkout_dir_set()
 
-  def get(self, name: NamedBasePathsType) -> config_types.Path:
+  def get(self, name: NamedBasePathsType, *,
+          skip_deprecation=False) -> config_types.Path:
     """Gets the base path named `name`. See module docstring for more info.
 
     DEPRECATED: Use the following @properties on this module instead:
@@ -664,15 +667,14 @@
       * checkout_dir (but use of checkout_dir is generally discouraged - just
       pass the Paths around instead of using this global variable).
     """
+    if not skip_deprecation:
+      self.m.warning.issue('PATH_GETITEM_DEPRECATED')
+
     match name:
       case 'cache':
         return self.cache_dir
       case 'checkout':
-        if cdir := self.checkout_dir:
-          # If the checkout_dir is already set, just return it directly.
-          return cdir
-        # In this case, the checkout_dir is not yet set, but it could be later.
-        return config_types.Path(config_types.CheckoutBasePath())
+        return self.checkout_dir
       case 'cleanup':
         return self.cleanup_dir
       case 'home':
@@ -697,7 +699,7 @@
       pass the Paths around instead of using this global variable).
     """
     self.m.warning.issue('PATH_GETITEM_DEPRECATED')
-    return self.get(name)
+    return self.get(name, skip_deprecation=True)
 
   @property
   def start_dir(self) -> config_types.Path:
diff --git a/recipe_modules/path/examples/full.py b/recipe_modules/path/examples/full.py
index 1b8ac3e..4c7c3dd 100644
--- a/recipe_modules/path/examples/full.py
+++ b/recipe_modules/path/examples/full.py
@@ -15,7 +15,8 @@
 from builtins import range, zip
 
 
-@recipe_api.ignore_warnings('recipe_engine/CHECKOUT_DIR_DEPRECATED')
+@recipe_api.ignore_warnings('recipe_engine/CHECKOUT_DIR_DEPRECATED',
+                            'recipe_engine/PATH_GETITEM_DEPRECATED')
 def RunSteps(api):
   api.step('step1', ['/bin/echo', str(api.path.tmp_base_dir / 'foo')])
 
diff --git a/recipe_modules/path/tests/dynamic_paths.py b/recipe_modules/path/tests/dynamic_paths.py
index 2fe2721..7421d44 100644
--- a/recipe_modules/path/tests/dynamic_paths.py
+++ b/recipe_modules/path/tests/dynamic_paths.py
@@ -17,7 +17,7 @@
   try:
     # Note - legacy api.path.get('checkout') is the only way to get a dynamic
     # checkout path before assignment to checkout_dir.
-    api.path.checkout_dir = api.path.get('checkout') / 'something'
+    api.path.checkout_dir = api.path.checkout_dir / 'something'
     assert False, 'able to assign string to path?'  # pragma: no cover
   except ValueError as ex:
     assert 'cannot be rooted in checkout_dir' in str(ex), str(ex)
diff --git a/recipe_modules/path/tests/test_api_legacy.py b/recipe_modules/path/tests/test_api_legacy.py
index 9199b7e..b4da7d8 100644
--- a/recipe_modules/path/tests/test_api_legacy.py
+++ b/recipe_modules/path/tests/test_api_legacy.py
@@ -7,31 +7,26 @@
 
 DEPS = ['path']
 
-GETITEM_NAMES = [
-    'cache',
-    'cleanup',
-    'home',
+GETATTR_NAMES = [
+    'cache_dir',
+    'cleanup_dir',
+    'home_dir',
     'start_dir',
-    'tmp_base',
+    'tmp_base_dir',
 ]
 
 
 def RunSteps(api):
-  for name in GETITEM_NAMES:
-    p = api.path.get(name) / 'file'
+  for name in GETATTR_NAMES:
+    p = getattr(api.path, name) / 'file'
     assert api.path.exists(p), p
 
   api.path.checkout_dir = api.path.start_dir / 'somedir'
-  assert api.path.exists(api.path.get('checkout') / 'file')
+  assert api.path.exists(getattr(api.path, 'checkout_dir') / 'file')
 
 
 def GenTests(api):
-  def base_path(name):
-    if not name.endswith('_dir'):
-      name += '_dir'
-    return getattr(api.path, name)
-
-  paths = [base_path(name) / 'file' for name in GETITEM_NAMES]
+  paths = [getattr(api.path, name) / 'file' for name in GETATTR_NAMES]
   paths.append(api.path.checkout_dir / 'file')
 
   yield api.test(