fix(update): one bad workspaces glob no longer aborts the lockfile-churn cleanup
Path.glob raises NotImplementedError for a non-relative pattern, which a string `workspaces` (iterated char by char, so "/") or an absolute entry produces. The (OSError, ValueError, TypeError) catch missed it, so the error escaped to the caller's suppress(Exception) and no lock was reverted at all -- back to autostash every run. Non-list values are now ignored and each pattern is tried on its own so a bad one just owns nothing. install.sh: read the workspace globs with `while read` instead of an unquoted $(...) so they are never pathname-expanded against the caller's CWD before `case` sees the pattern.
This commit is contained in:
@@ -432,10 +432,15 @@ def _npm_lockfile_owners(repo_root: Path) -> set[Path]:
|
||||
workspaces = package.get("workspaces", [])
|
||||
if isinstance(workspaces, dict):
|
||||
workspaces = workspaces.get("packages", [])
|
||||
if not isinstance(workspaces, list):
|
||||
return owners
|
||||
for pattern in workspaces:
|
||||
for directory in repo_root.glob(str(pattern)):
|
||||
if (directory / "package.json").is_file():
|
||||
owners.add(directory.relative_to(repo_root))
|
||||
# One bad glob (absolute pattern -> NotImplementedError) degrades to "not an owner"
|
||||
# instead of aborting the whole churn cleanup through the caller's suppress(Exception).
|
||||
with suppress(Exception):
|
||||
for directory in repo_root.glob(str(pattern)):
|
||||
if (directory / "package.json").is_file():
|
||||
owners.add(directory.relative_to(repo_root))
|
||||
except (OSError, ValueError, TypeError):
|
||||
pass
|
||||
return owners
|
||||
|
||||
@@ -296,13 +296,20 @@ discard_update_lockfile_churn() {
|
||||
if [ "$pkg_dir" = "." ]; then
|
||||
root_lock_protected=1
|
||||
else
|
||||
# Read the globs line by line: an unquoted $(...) would pathname-expand
|
||||
# them against the caller's CWD before `case` ever sees the pattern.
|
||||
# `case` globs match across "/", so a manifest nested under a workspace
|
||||
# also protects the root lock (fail-safe; Python matches one level).
|
||||
local ws_glob
|
||||
for ws_glob in $(sed -n '/"workspaces"[[:space:]]*:/,/\]/p' "$repo/package.json" 2>/dev/null \
|
||||
| grep -o '"[^"]*"' | tr -d '"' | grep -v -e '^workspaces$' -e '^packages$'); do
|
||||
while IFS= read -r ws_glob; do
|
||||
[ -n "$ws_glob" ] || continue
|
||||
case "$pkg_dir" in
|
||||
$ws_glob) root_lock_protected=1 ;;
|
||||
esac
|
||||
done
|
||||
done <<WS_EOF
|
||||
$(sed -n '/"workspaces"[[:space:]]*:/,/\]/p' "$repo/package.json" 2>/dev/null \
|
||||
| grep -o '"[^"]*"' | tr -d '"' | grep -v -e '^workspaces$' -e '^packages$')
|
||||
WS_EOF
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
|
||||
@@ -53,3 +53,14 @@ def test_manifest_outside_workspace_graph_does_not_protect_root_lock(tmp_path):
|
||||
assert _still_dirty_after_cleanup(repo, "vendor/foo/package.json", "package-lock.json") == {
|
||||
"vendor/foo/package.json",
|
||||
}
|
||||
|
||||
|
||||
def test_unsupported_workspaces_glob_still_discards_root_lock_churn(tmp_path):
|
||||
"""A string ``workspaces`` (iterated char by char, '/' is a non-relative glob) must not abort
|
||||
the cleanup: the churned root lock is still reverted, the bad entry just owns nothing."""
|
||||
repo = _repo(tmp_path)
|
||||
(repo / "package.json").write_text(json.dumps({"workspaces": "apps/*"}), encoding="utf-8")
|
||||
_git(repo, "commit", "-qam", "string workspaces")
|
||||
assert _still_dirty_after_cleanup(repo, "vendor/foo/package.json", "package-lock.json") == {
|
||||
"vendor/foo/package.json",
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user