[PATCH 0/4] oe/path: three fixes plus selftest coverage

[PATCH 0/4] oe/path: three fixes plus selftest coverage

[Trevor Woerner]

While adding unit coverage for the copy of oe/path.py vendored into the
standalone wic repository, three real bugs turned up in the shared
meta/lib/oe/path.py. They are fixed here at the source so the whole
project benefits, each as its own standalone commit:

  - __realpath()'s isdir guard assigns a bare `false`, which is not a
    Python name; when os.path.isdir() raises, the handler meant to
    absorb the error raises NameError instead. Use the builtin False.
  - symlink(force=True) cleared the destination through remove(), which
    globs its argument; a destination containing glob metacharacters
    could fail to match itself (leaving a stale entry) or match
    unrelated files. Remove the literal destination instead.
  - canonicalize('') and canonicalize(None) returned the current
    working directory, because os.path.realpath('') does; a stray
    separator injected a spurious cwd entry too. Skip empty tokens.

The final commit extends meta/lib/oeqa/selftest/cases/liboe.py with a
PathTests class covering oe.path's own logic (join(), is_path_parent(),
symlink(), make_relative_symlink(), canonicalize(), which_wild(),
realpath()) and locking in the three fixes; backing any fix out turns
the matching test red. oe-selftest -r liboe.PathTests runs 37 tests,
all passing.

Trevor Woerner (4):
  oe/path: fix bare `false` NameError in __realpath's isdir guard
  oe/path: don't glob-expand the destination in symlink(force=True)
  oe/path: canonicalize('') should return '' rather than the cwd
  oeqa/selftest/liboe: cover oe.path's own path logic

 meta/lib/oe/path.py                   |  21 +-
 meta/lib/oeqa/selftest/cases/liboe.py | 289 ++++++++++++++++++++++++++
 2 files changed, 307 insertions(+), 3 deletions(-)

-- 
2.50.0.173.g8b6f19ccfc3a

[PATCH 1/4] oe/path: fix bare `false` NameError in __realpath's isdir guard

[Trevor Woerner]

__realpath() wraps its os.path.isdir() probe in a bare except that
assigns `is_dir = false`. `false` is not a Python name, so when
os.path.isdir() does raise (for example on an ELOOP path), the handler
meant to absorb the error instead raises NameError and aborts the walk.

Use the builtin False so the guard degrades to "not a directory" as
intended.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 meta/lib/oe/path.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index c4588ad0e65a..f0462d276196 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -234,7 +234,7 @@ def __realpath(file, root, loop_cnt, assume_dir):
     try:
         is_dir = os.path.isdir(file)
     except:
-        is_dir = false
+        is_dir = False
 
     return (file, is_dir)
 
-- 
2.50.0.173.g8b6f19ccfc3a

[PATCH 2/4] oe/path: don't glob-expand the destination in symlink(force=True)

[Trevor Woerner]

symlink(source, destination, force=True) cleared an existing destination
by calling remove(destination). remove() treats its argument as a glob
pattern (it iterates glob.glob(path)), so a destination whose name
contains glob metacharacters is mishandled: a name such as "foo[bar]"
may fail to match itself and be left in place, or a pattern could match
and delete unrelated files.

Remove the literal destination instead: unlink it directly, and fall
back to rmtree() for a directory, ignoring ENOENT. This keeps the
force=True semantics without passing the path through glob.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 meta/lib/oe/path.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index f0462d276196..44a9ff9d0757 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -169,7 +169,18 @@ def symlink(source, destination, force=False):
     """Create a symbolic link"""
     try:
         if force:
-            remove(destination)
+            # Remove the exact destination path. Do not route this through
+            # remove(), which treats its argument as a glob pattern: a
+            # destination containing glob metacharacters (for example a
+            # '[' in the name) could fail to match, or match and delete
+            # unrelated files.
+            try:
+                os.unlink(destination)
+            except OSError as exc:
+                if exc.errno == errno.EISDIR:
+                    shutil.rmtree(destination)
+                elif exc.errno != errno.ENOENT:
+                    raise
         os.symlink(source, destination)
     except OSError as e:
         if e.errno != errno.EEXIST or os.readlink(destination) != source:
-- 
2.50.0.173.g8b6f19ccfc3a

[PATCH 3/4] oe/path: canonicalize('') should return '' rather than the cwd

[Trevor Woerner]

canonicalize() splits its input on the separator and runs each token
through os.path.realpath(). os.path.realpath('') returns the current
working directory, so canonicalize('') and canonicalize(None) wrongly
produced the cwd instead of an empty string, and a stray separator (for
example "a,,b") injected a spurious cwd entry into the result.

Skip empty tokens alongside the existing unexpanded-variable skip so only
real paths are canonicalized.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 meta/lib/oe/path.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index 44a9ff9d0757..7255d99bc2f7 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -356,7 +356,11 @@ def canonicalize(paths, sep=','):
     # prefixes in sting compares later on, where the slashes then are important.
     canonical_paths = []
     for path in (paths or '').split(sep):
-        if '$' not in path:
+        # Skip empty tokens as well as unexpanded bitbake variables: an
+        # empty path would otherwise reach os.path.realpath(''), which
+        # returns the current working directory, so canonicalize('') or
+        # canonicalize(None) would wrongly produce the cwd instead of ''.
+        if path and '$' not in path:
             trailing_slash = path.endswith('/') and '/' or ''
             canonical_paths.append(os.path.realpath(path) + trailing_slash)
 
-- 
2.50.0.173.g8b6f19ccfc3a

[PATCH 4/4] oeqa/selftest/liboe: cover oe.path's own path logic

[Trevor Woerner]

Add in-process coverage for oe.path's join(), is_path_parent(),
symlink(), make_relative_symlink(), canonicalize(), which_wild() and
realpath(), alongside the existing copytree tests.

The cases pin behaviour that is easy to regress: __realpath() falls back
to "not a directory" when os.path.isdir() raises; symlink(force=True)
replaces the literal destination even when its name contains glob
metacharacters, and never removes a pattern sibling; and canonicalize()
skips empty tokens so '', None, and a stray separator do not inject the
current working directory.

AI-Generated: codex/claude-opus 4.8 (xhigh)
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 meta/lib/oeqa/selftest/cases/liboe.py | 289 ++++++++++++++++++++++++++
 1 file changed, 289 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/liboe.py b/meta/lib/oeqa/selftest/cases/liboe.py
index 930354c931a3..fa4902610cc4 100644
--- a/meta/lib/oeqa/selftest/cases/liboe.py
+++ b/meta/lib/oeqa/selftest/cases/liboe.py
@@ -8,6 +8,8 @@ from oeqa.selftest.case import OESelftestTestCase
 from oeqa.utils.commands import get_bb_var, get_bb_vars, bitbake, runCmd
 import oe.path
 import os
+import tempfile
+import unittest.mock
 
 class CopyTreeTests(OESelftestTestCase):
 
@@ -135,3 +137,290 @@ exit 42
         self.assertIn("exit status 42", string)
         self.assertIn("Standard Output: Via stdout", string)
         self.assertIn("Standard Error: Via stderr", string)
+
+class PathTests(OESelftestTestCase):
+    """
+    In-process coverage for oe.path's own path logic: join, is_path_parent,
+    symlink, make_relative_symlink, canonicalize, which_wild and realpath.
+    Thin standard-library wrappers (relative -> os.path.relpath,
+    find -> os.walk) are left to the standard library.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self._tmp = tempfile.TemporaryDirectory(prefix="oeqa-oepath-")
+        self.addCleanup(self._tmp.cleanup)
+        self.tmp_path = self._tmp.name
+
+    # --- join --------------------------------------------------------------
+    # join() is os.path.normpath("/".join(paths)); unlike os.path.join it
+    # does not treat an absolute right-hand component specially.
+
+    def test_join_two_paths(self):
+        self.assertEqual(oe.path.join("a", "b"), "a/b")
+
+    def test_join_absolute_rhs_is_not_special(self):
+        # os.path.join("a", "/b") == "/b"; oe.path.join() keeps it relative.
+        self.assertEqual(oe.path.join("a", "/b"), "a/b")
+
+    def test_join_redundant_separators_normalised(self):
+        self.assertEqual(oe.path.join("a//", "b"), "a/b")
+
+    def test_join_pardir_normalised(self):
+        self.assertEqual(oe.path.join("a", "..", "b"), "b")
+
+    def test_join_empty_leading_component_is_absolute(self):
+        # "/".join(["", "b"]) is "/b", which normpath leaves absolute.
+        self.assertEqual(oe.path.join("", "b"), "/b")
+
+    # --- is_path_parent ----------------------------------------------------
+
+    def test_is_path_parent_direct_child(self):
+        self.assertTrue(oe.path.is_path_parent("/usr", "/usr/bin"))
+
+    def test_is_path_parent_unrelated_path(self):
+        self.assertFalse(oe.path.is_path_parent("/usr", "/tmp"))
+
+    def test_is_path_parent_child_is_not_parent_of_its_parent(self):
+        self.assertFalse(oe.path.is_path_parent("/usr/bin", "/usr"))
+
+    def test_is_path_parent_prefix_string_is_not_a_path_parent(self):
+        # "/usrlocal" shares a string prefix with "/usr" but is not below it;
+        # the trailing-separator handling must reject it.
+        self.assertFalse(oe.path.is_path_parent("/usr", "/usrlocal"))
+
+    def test_is_path_parent_no_paths_returns_false(self):
+        self.assertFalse(oe.path.is_path_parent("/usr"))
+
+    def test_is_path_parent_all_paths_must_be_below(self):
+        self.assertTrue(oe.path.is_path_parent("/usr", "/usr/bin", "/usr/lib"))
+
+    def test_is_path_parent_one_path_outside_fails(self):
+        self.assertFalse(oe.path.is_path_parent("/usr", "/usr/bin", "/tmp"))
+
+    # --- symlink -----------------------------------------------------------
+
+    def test_symlink_creates_symlink(self):
+        src = os.path.join(self.tmp_path, "target.txt")
+        with open(src, "w") as f:
+            f.write("content")
+        dst = os.path.join(self.tmp_path, "link")
+        oe.path.symlink(src, dst)
+        self.assertTrue(os.path.islink(dst))
+        self.assertEqual(os.readlink(dst), src)
+
+    def test_symlink_matching_existing_is_idempotent(self):
+        src = os.path.join(self.tmp_path, "target.txt")
+        with open(src, "w") as f:
+            f.write("content")
+        dst = os.path.join(self.tmp_path, "link")
+        oe.path.symlink(src, dst)
+        oe.path.symlink(src, dst)
+        self.assertEqual(os.readlink(dst), src)
+
+    def test_symlink_conflicting_existing_raises(self):
+        src1 = os.path.join(self.tmp_path, "t1.txt")
+        src2 = os.path.join(self.tmp_path, "t2.txt")
+        for p in (src1, src2):
+            with open(p, "w") as f:
+                f.write(p)
+        dst = os.path.join(self.tmp_path, "link")
+        oe.path.symlink(src1, dst)
+        with self.assertRaises(OSError):
+            oe.path.symlink(src2, dst)
+
+    def test_symlink_force_overwrites_existing_symlink(self):
+        src1 = os.path.join(self.tmp_path, "t1")
+        src2 = os.path.join(self.tmp_path, "t2")
+        for p in (src1, src2):
+            with open(p, "w") as f:
+                f.write(p)
+        dst = os.path.join(self.tmp_path, "link")
+        oe.path.symlink(src1, dst)
+        oe.path.symlink(src2, dst, force=True)
+        self.assertEqual(os.readlink(dst), src2)
+
+    def test_symlink_force_replaces_destination_with_glob_metacharacters(self):
+        # A destination whose name contains glob metacharacters must still be
+        # replaced. Routing the removal through remove() (which globs) fails
+        # to match "link[1]" against itself, leaving the stale entry in place.
+        src = os.path.join(self.tmp_path, "target")
+        with open(src, "w") as f:
+            f.write("content")
+        dst = os.path.join(self.tmp_path, "link[1]")
+        with open(dst, "w") as f:
+            f.write("stale regular file")
+        oe.path.symlink(src, dst, force=True)
+        self.assertTrue(os.path.islink(dst))
+        self.assertEqual(os.readlink(dst), src)
+
+    def test_symlink_force_does_not_delete_glob_siblings(self):
+        # The destination "keep?.txt" does not exist, but a sibling
+        # "keepX.txt" matches it as a glob pattern. force=True must remove
+        # only the literal destination, never a pattern sibling.
+        sibling = os.path.join(self.tmp_path, "keepX.txt")
+        with open(sibling, "w") as f:
+            f.write("do not delete me")
+        src = os.path.join(self.tmp_path, "target")
+        with open(src, "w") as f:
+            f.write("content")
+        dst = os.path.join(self.tmp_path, "keep?.txt")
+        oe.path.symlink(src, dst, force=True)
+        self.assertTrue(os.path.exists(sibling))
+        self.assertEqual(os.readlink(dst), src)
+
+    def test_symlink_force_replaces_existing_directory(self):
+        # An existing directory at the destination is torn down (EISDIR ->
+        # rmtree) before the link is created.
+        src = os.path.join(self.tmp_path, "target")
+        with open(src, "w") as f:
+            f.write("content")
+        dst = os.path.join(self.tmp_path, "dir")
+        os.mkdir(dst)
+        with open(os.path.join(dst, "child"), "w") as f:
+            f.write("x")
+        oe.path.symlink(src, dst, force=True)
+        self.assertTrue(os.path.islink(dst))
+        self.assertEqual(os.readlink(dst), src)
+
+    # --- make_relative_symlink ---------------------------------------------
+
+    def test_make_relative_symlink_non_symlink_is_ignored(self):
+        regular = os.path.join(self.tmp_path, "file.txt")
+        with open(regular, "w") as f:
+            f.write("content")
+        oe.path.make_relative_symlink(regular)
+        self.assertTrue(os.path.isfile(regular))
+
+    def test_make_relative_symlink_already_relative_unchanged(self):
+        target = os.path.join(self.tmp_path, "target.txt")
+        with open(target, "w") as f:
+            f.write("content")
+        link = os.path.join(self.tmp_path, "link")
+        os.symlink("target.txt", link)
+        oe.path.make_relative_symlink(link)
+        self.assertEqual(os.readlink(link), "target.txt")
+
+    def test_make_relative_symlink_absolute_becomes_relative_and_resolves(self):
+        target = os.path.join(self.tmp_path, "target.txt")
+        with open(target, "w") as f:
+            f.write("content")
+        link = os.path.join(self.tmp_path, "link")
+        os.symlink(target, link)
+        self.assertTrue(os.path.isabs(os.readlink(link)))
+        oe.path.make_relative_symlink(link)
+        result = os.readlink(link)
+        self.assertFalse(os.path.isabs(result))
+        self.assertTrue(os.path.exists(link))
+        with open(link) as f:
+            self.assertEqual(f.read(), "content")
+
+    # --- canonicalize ------------------------------------------------------
+
+    def test_canonicalize_real_path_returned(self):
+        self.assertEqual(oe.path.canonicalize(self.tmp_path),
+                         os.path.realpath(self.tmp_path))
+
+    def test_canonicalize_unexpanded_variable_is_skipped(self):
+        self.assertEqual(oe.path.canonicalize("$SOME_VAR"), "")
+
+    def test_canonicalize_variable_token_dropped_leaving_real_path(self):
+        result = oe.path.canonicalize("$VAR," + self.tmp_path)
+        # The "$VAR" token is dropped entirely: no empty placeholder, no
+        # leading separator, just the canonical real path.
+        self.assertEqual(result, os.path.realpath(self.tmp_path))
+
+    def test_canonicalize_empty_string_returns_empty_string(self):
+        # os.path.realpath("") is the cwd; canonicalize("") must not leak it.
+        self.assertEqual(oe.path.canonicalize(""), "")
+
+    def test_canonicalize_none_returns_empty_string(self):
+        self.assertEqual(oe.path.canonicalize(None), "")
+
+    def test_canonicalize_empty_token_between_paths_is_dropped(self):
+        p1 = os.path.join(self.tmp_path, "a")
+        p2 = os.path.join(self.tmp_path, "b")
+        os.mkdir(p1)
+        os.mkdir(p2)
+        # The stray separator in "a,,b" must not inject a cwd entry.
+        result = oe.path.canonicalize("%s,,%s" % (p1, p2))
+        self.assertEqual(result, "%s,%s" % (os.path.realpath(p1),
+                                            os.path.realpath(p2)))
+
+    def test_canonicalize_trailing_slash_preserved(self):
+        result = oe.path.canonicalize(self.tmp_path + "/")
+        self.assertEqual(result, os.path.realpath(self.tmp_path) + "/")
+
+    # --- which_wild --------------------------------------------------------
+
+    def test_which_wild_missing_tool_returns_empty(self):
+        self.assertEqual(oe.path.which_wild("totally_nonexistent_tool_xyz"), [])
+
+    def test_which_wild_explicit_search_path(self):
+        tool = os.path.join(self.tmp_path, "mytool")
+        with open(tool, "w") as f:
+            f.write("#!/bin/sh\n")
+        os.chmod(tool, 0o755)
+        self.assertEqual(oe.path.which_wild("mytool", path=self.tmp_path),
+                         [tool])
+
+    def test_which_wild_wildcard_pattern(self):
+        for name in ("foo-a", "foo-b", "bar"):
+            with open(os.path.join(self.tmp_path, name), "w") as f:
+                f.write("")
+        results = oe.path.which_wild("foo-*", path=self.tmp_path)
+        self.assertEqual(sorted(os.path.basename(p) for p in results),
+                         ["foo-a", "foo-b"])
+
+    def test_which_wild_first_match_per_name_wins(self):
+        # A name found in an earlier PATH element shadows the same name later;
+        # reverse=True walks PATH the other way, so the other copy wins.
+        first = os.path.join(self.tmp_path, "first")
+        second = os.path.join(self.tmp_path, "second")
+        os.mkdir(first)
+        os.mkdir(second)
+        for d in (first, second):
+            with open(os.path.join(d, "tool"), "w") as f:
+                f.write("")
+        search = "%s:%s" % (first, second)
+        self.assertEqual(oe.path.which_wild("tool", path=search),
+                         [os.path.join(first, "tool")])
+        self.assertEqual(oe.path.which_wild("tool", path=search, reverse=True),
+                         [os.path.join(second, "tool")])
+
+    # --- realpath ----------------------------------------------------------
+
+    def test_realpath_resolves_symlink_below_root(self):
+        target = os.path.join(self.tmp_path, "real")
+        os.mkdir(target)
+        link = os.path.join(self.tmp_path, "lnk")
+        os.symlink("real", link)
+        self.assertEqual(oe.path.realpath(link, self.tmp_path), target)
+
+    def test_realpath_plain_path_is_returned(self):
+        sub = os.path.join(self.tmp_path, "d")
+        os.mkdir(sub)
+        self.assertEqual(oe.path.realpath(sub, self.tmp_path), sub)
+
+    def test_realpath_path_outside_root_raises(self):
+        root = os.path.join(self.tmp_path, "root")
+        outside = os.path.join(self.tmp_path, "outside")
+        os.mkdir(root)
+        os.mkdir(outside)
+        with self.assertRaises(OSError):
+            oe.path.realpath(outside, root)
+
+    def test_realpath_isdir_failure_falls_back_without_nameerror(self):
+        # The final stanza of __realpath tolerates any failure from
+        # os.path.isdir by treating the path as not-a-directory. Force isdir
+        # to raise and confirm the fallback returns the resolved path rather
+        # than raising NameError from an undefined fallback value.
+        sub = os.path.join(self.tmp_path, "d")
+        os.mkdir(sub)
+
+        def boom(_path):
+            raise OSError("synthetic stat failure")
+
+        with unittest.mock.patch("os.path.isdir", boom):
+            self.assertEqual(
+                oe.path.realpath(sub, self.tmp_path, use_physdir=False), sub)
-- 
2.50.0.173.g8b6f19ccfc3a