From c47060fd6379ff7a04d703dc780ae7dd01012962 Mon Sep 17 00:00:00 2001 From: "picknik-johnny-5-bot[bot]" <311204565+picknik-johnny-5-bot[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:29:13 +0000 Subject: [PATCH 1/4] feat(deps): add explicit refresh from upstream tags --- README.md | 53 +- bin/refresh_workspace_dependencies.py | 421 ++++++++++ .../test_refresh_workspace_dependencies.py | 722 ++++++++++++++++++ bin/validate_workspace_dependencies.py | 27 + 4 files changed, 1217 insertions(+), 6 deletions(-) create mode 100644 bin/refresh_workspace_dependencies.py create mode 100644 bin/tests/test_refresh_workspace_dependencies.py diff --git a/README.md b/README.md index c28c48715..528f01630 100644 --- a/README.md +++ b/README.md @@ -46,15 +46,56 @@ The hardware-only `kinova_gen3_site_config` and `picknik_ur_site_config` configu ## Updating vendored dependencies -Each `UPSTREAM.yaml` under `src/external_dependencies` records the exact upstream commit and retained paths. To refresh one: check the tree out at the new commit, preserve its license files, reapply the pruning described in `pruning_notes`, and validate every config that consumes the package. +Each `UPSTREAM.yaml` under `src/external_dependencies` records the upstream repository, branch, exact commit, retained paths, and local modifications. Refresh these snapshots explicitly from the repository root. You need Python 3.10+, Git, Git LFS, network access to the configured repositories, and temporary disk space for upstream Git history, fetched LFS assets, and a staged dependency copy. Commit or stash changes in the dependencies you select first, including untracked and ignored files. Run `git lfs pull` if your checkout still contains LFS pointers instead of asset bytes. -Then update `commit` and the retained-path lists, and check the result: +Preview all dependencies, then apply only the one you want: ```bash -python3 bin/validate_workspace_dependencies.py # structure, runs on every PR -python3 bin/validate_workspace_dependencies.py --verify-upstream # fetches the pinned commit and compares files +python3 bin/validate_workspace_dependencies.py --refresh-from-upstream all --dry-run +python3 bin/validate_workspace_dependencies.py --refresh-from-upstream feetech_ros2_driver --dry-run +python3 bin/validate_workspace_dependencies.py --refresh-from-upstream feetech_ros2_driver ``` -The second command needs network access. Run it manually after re-vendoring. CI checks structure only; it does not compare vendored contents or create upstream-verification issues. +`feetech_ros2_driver` is the **dependency directory name** under `src/external_dependencies`, not a robot config or its dependency graph. The example may report `already current`; it is not a promise that a newer eligible tag exists. To apply every eligible refresh instead: -The optional ML model submodules can be advanced independently when their demonstration Objectives need a newer model package. +```bash +python3 bin/validate_workspace_dependencies.py --refresh-from-upstream all +``` + +These commands fetch upstream data and change only local vendored files and the manifest's `commit` field. They never commit, push, create a PR, or update the optional ML submodules. `--dry-run` performs the same selection and merge checks without writing the snapshot. A later apply fetches tags again, so review the reported tag and commit. + +### Which tag is selected? + +The refresher selects the highest numeric `MAJOR.MINOR.PATCH` tag, optionally prefixed with `v`, whose commit is both reachable from the configured branch and contains the current pinned commit in its ancestry. Annotated and lightweight tags work. Prereleases, build suffixes, and other naming conventions are excluded. The version cannot be lower than the highest stable tag reachable from the current pin. Equal versions with different eligible commits are ambiguous and require manual selection. + +This is deliberately stricter than choosing the newest tag by date or following branch HEAD. In particular, a tag predating PickNik fork commits cannot replace those patches. A pin already at the selected tag is unchanged. If there is no eligible tag, the dependency is left untouched with an error explaining that it needs manual review. There is **no branch-HEAD fallback**. Forks such as `main-picknik` or `ros2-fix-deps` may have no qualifying release. + +### Local patches, pruning, and manual cases + +For an unchanged retained-file inventory (excluding demonstrably already-pruned subtrees), the refresher uses Git's three-way text merge with the old upstream pin as the base. It preserves committed local additions and existing pruned files, honors `snapshot_path`, and keeps manifest comments and annotations byte-for-byte except for the commit field. It checks the modification ledger before and after merging, and validates the complete proposed snapshot and manifest in temporary staging before writing. Incoming license changes that need new metadata stop the refresh for manual review. + +The manifest's `pruning_notes` are prose, not executable selection rules. Changes wholly inside an old upstream directory that is entirely absent locally stay pruned. Other added, deleted, renamed, or mode/type-changed upstream paths inside a retained boundary require a **manual refresh**, rather than guessing which new files to include or silently dropping files. Other manual cases include merge conflicts, symlinks, divergent local binary patches, unsupported LFS-to-ordinary-file transitions, and patches absorbed upstream that need a `modified_paths` correction. Changed upstream LFS assets can update unmodified local assets: Git LFS fetches actual bytes into temporary storage, and the refresher verifies their SHA-256 and size against the selected pointer. Missing or invalid objects stop the dependency; pointers are never installed. Unchanged bytes are not rewritten. + +Clean CRLF checkout conversion is not normalized automatically. Use an LF checkout after reviewing `core.autocrlf` and text/EOL attributes, or refresh manually; do not record checkout conversion as a local patch. + +A selection, merge, LFS retrieval, or proposed-validation error leaves that dependency untouched. Writes are not crash-atomic: an interruption or write-time disk failure can leave a partial update. In `all` mode, independent successful dependencies can still be refreshed; the command exits nonzero if any dependency fails. Inspect each reported result. For a manual refresh, check out the chosen upstream commit separately, preserve licenses and notices, reapply local patches and pruning, and update the manifest's commit and path declarations. Do not change branches merely to bypass a missing tag or discard fork patches. + +### Review and publish the local changes + +```bash +git diff --stat +git diff -- src/external_dependencies +python3 bin/validate_workspace_dependencies.py # offline structure check +python3 bin/validate_workspace_dependencies.py --verify-upstream # read-only comparison with each pinned upstream commit +git diff --check +``` + +Review release notes, license changes, and any provenance notes that mention an older release, then build and test every robot config that consumes the changed packages. The refresher does not prove runtime compatibility. After review and validation, commit and push through your normal PR workflow, for example for a single dependency: + +```bash +git add src/external_dependencies/feetech_ros2_driver +git commit -m "Refresh vendored Feetech driver dependency" +git push +``` + +`--verify-upstream` needs network access but does not refresh files. CI checks structure only; it does not compare vendored contents, schedule refreshes, or create upstream-verification issues. The optional ML model submodules can be advanced independently when their demonstration Objectives need a newer model package. diff --git a/bin/refresh_workspace_dependencies.py b/bin/refresh_workspace_dependencies.py new file mode 100644 index 000000000..7790ac831 --- /dev/null +++ b/bin/refresh_workspace_dependencies.py @@ -0,0 +1,421 @@ +"""Explicit, local-only refresh of vendored snapshots from stable upstream tags.""" + +from pathlib import Path +import hashlib +import os +import re +import subprocess +import sys +import tempfile + +import validate_workspace_dependencies as validator + +STABLE_TAG = re.compile(r"v?(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)") + + +def git(root: Path, *args: str) -> bytes: + environment = os.environ.copy() + # Keep authentication and URL rewrites, not the invoking alias's repository. + for key in ( + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_COMMON_DIR", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_PREFIX", + "GIT_CEILING_DIRECTORIES", + "GIT_DISCOVERY_ACROSS_FILESYSTEM", + ): + environment.pop(key, None) + return subprocess.run( + ["git", "-C", str(root), *args], + check=True, + capture_output=True, + timeout=300, + env=environment, + ).stdout + + +def content_matches(local: bytes, upstream: bytes) -> bool: + """Compare actual bytes with upstream content or its LFS object identity.""" + if pointer := validator.LFS_POINTER_OID.fullmatch(upstream): + return (hashlib.sha256(local).hexdigest().encode(), len(local)) == ( + pointer[1], + int(pointer[2]), + ) + return local == upstream + + +def check_ledger(path: Path, local: bytes, upstream: bytes, manifest: dict) -> None: + if (not content_matches(local, upstream)) != ( + path.as_posix() in manifest.get("modified_paths", []) + ): + raise ValueError( + f"modified_paths disagrees at {path}; reconcile the ledger manually (including absorbed patches)" + ) + if ( + not content_matches(local, upstream) + and validator.path_is_declared( + path, {Path(p) for p in manifest.get("apache_paths", [])} + ) + and not validator.path_is_declared( + path, {Path(p) for p in manifest.get("apache_excluded_paths", [])} + ) + and validator.PICKNIK_MODIFICATION_NOTICE not in local + ): + raise ValueError( + f"missing PickNik modification notice at {path}; review the Apache declaration manually" + ) + + +def require_clean(manifest_path: Path) -> None: + """Do not fold uncommitted work into a refresh.""" + if git( + manifest_path.parent, + "status", + "--porcelain", + "--untracked-files=all", + "--ignored", + "--", + ".", + ): + raise ValueError( + "dirty dependency; commit or stash its changes (including ignored files) before refreshing" + ) + + +def refresh_one(manifest_path: Path, *, dry_run: bool = False) -> None: + require_clean(manifest_path) + # Refresh never follows symlinks, even ones the read-only validator accepts. + for path in [ + manifest_path, + *manifest_path.parents, + *manifest_path.parent.rglob("*"), + ]: + if path.is_symlink(): + raise ValueError( + f"unsafe symlink for automatic refresh: {path}; refresh manually" + ) + if errors := validator.validate_vendor_manifest(manifest_path): + raise ValueError("; ".join(errors)) + manifest = validator.parse_vendor_manifest(manifest_path) + retained = {Path(value) for value in manifest["vendored_paths"]} + for path in manifest_path.parent.rglob("*"): + if ( + path.is_file() + and path != manifest_path + and not validator.path_is_declared( + path.relative_to(manifest_path.parent), retained + ) + ): + raise ValueError( + f"{path} is outside vendored_paths; reconcile the manifest manually" + ) + upstream = manifest["upstream"] + old = upstream["commit"] + with tempfile.TemporaryDirectory(prefix="workspace-refresh-") as temporary: + repository = Path(temporary) + git(repository, "init", "--quiet") + git( + repository, + "fetch", + "--quiet", + "--tags", + upstream["repository"], + f"+refs/heads/{upstream['branch']}:refs/remotes/upstream/branch", + ) + git( + repository, + "merge-base", + "--is-ancestor", + old, + "refs/remotes/upstream/branch", + ) + tags = ( + git( + repository, + "tag", + "--merged", + "refs/remotes/upstream/branch", + "--contains", + old, + ) + .decode() + .splitlines() + ) + prior_tags = git(repository, "tag", "--merged", old).decode().splitlines() + floor = max( + ( + tuple(map(int, match.groups())) + for name in prior_tags + if (match := STABLE_TAG.fullmatch(name)) + ), + default=(0, 0, 0), + ) + versions = [ + (tuple(map(int, match.groups())), tag) + for tag in tags + if (match := STABLE_TAG.fullmatch(tag)) + and tuple(map(int, match.groups())) >= floor + ] + if not versions: + raise ValueError( + f"no eligible stable tag in {upstream['repository']} on branch {upstream['branch']} " + f"containing pin {old}; review the configured branch and refresh manually" + ) + version, tag = max(versions) + commits = { + git(repository, "rev-parse", f"refs/tags/{name}^{{commit}}") + .decode() + .strip() + for candidate_version, name in versions + if candidate_version == version + } + if len(commits) != 1: + raise ValueError( + "ambiguous stable version tags identify different commits; select and refresh manually" + ) + new = commits.pop() + require_clean(manifest_path) + snapshot = Path(manifest.get("snapshot_path", ".")) + boundaries = { + Path(value).relative_to(snapshot) for value in manifest["vendored_paths"] + } + inventories = [] + for revision in (old, new): + inventory = {} + for entry in git(repository, "ls-tree", "-rz", revision).split(b"\0"): + if not entry: + continue + metadata, name = entry.split(b"\t", 1) + relative = Path(name.decode()) + if validator.path_is_declared(relative, boundaries): + inventory[relative] = metadata.split()[0] + inventories.append(inventory) + # Only directories present upstream at the old pin and entirely absent + # locally prove prior subtree pruning. New siblings remain ambiguous. + pruned = { + parent + for path in inventories[0] + for parent in path.parents + if validator.path_is_declared(parent, boundaries) + and not (manifest_path.parent / snapshot / parent).exists() + } + changed = sorted( + path + for path in inventories[0].keys() | inventories[1].keys() + if inventories[0].get(path) != inventories[1].get(path) + and not validator.path_is_declared(path, pruned) + ) + if changed: + raise ValueError( + f"unsafe inventory change at {tag} ({new}): " + + ", ".join(str(path) for path in changed) + + "; pruning_notes cannot decide retention; refresh manually and review vendored_paths" + ) + proposed = {} + for boundary in manifest["vendored_paths"]: + target = manifest_path.parent / boundary + paths = target.rglob("*") if target.is_dir() else [target] + for path in paths: + if not path.is_file(): + continue + relative = path.relative_to(manifest_path.parent).relative_to(snapshot) + modes = [inventory.get(relative, b"") for inventory in inventories] + if modes == [b"", b""] and path.relative_to( + manifest_path.parent + ).as_posix() in manifest.get("modified_paths", []): + continue + for mode in modes: + if mode not in (b"100644", b"100755"): + raise ValueError( + f"unsafe or missing upstream file: {relative}; refresh manually" + ) + base = git(repository, "show", f"{old}:{relative.as_posix()}") + incoming = git(repository, "show", f"{new}:{relative.as_posix()}") + local = path.read_bytes() + for content in (base, incoming, local): + if content.startswith( + b"version https://git-lfs.github.com/spec/" + ) and not validator.LFS_POINTER_OID.fullmatch(content): + raise ValueError( + f"invalid LFS pointer at {relative}; refresh manually" + ) + if validator.LFS_POINTER_OID.fullmatch(local): + raise ValueError( + f"LFS pointer in local snapshot {relative}; run git lfs pull before refreshing" + ) + local_relative = path.relative_to(manifest_path.parent) + eol = git( + manifest_path.parent, + "ls-files", + "--eol", + "--", + local_relative.as_posix(), + ) + if b"i/lf " in eol and b"w/crlf " in eol: + raise ValueError( + f"unsupported clean CRLF checkout conversion at {local_relative}; " + "use an LF checkout (review core.autocrlf and text/eol attributes, " + "then re-check out the clean files), or refresh manually; " + "do not add checkout conversion to modified_paths" + ) + check_ledger(local_relative, local, base, manifest) + if any( + validator.LFS_POINTER_OID.fullmatch(content) + for content in (base, incoming) + ): + if base != incoming: + pointer = validator.LFS_POINTER_OID.fullmatch(incoming) + if not pointer or not content_matches(local, base): + raise ValueError( + f"divergent or unsupported LFS change at {relative}; refresh manually" + ) + storage = repository / "lfs-store" + try: + git( + repository, + "config", + "remote.origin.url", + upstream["repository"], + ) + git(repository, "config", "lfs.storage", str(storage)) + git( + repository, + "lfs", + "fetch", + f"--include={relative.as_posix()}", + "--exclude=", + "origin", + new, + ) + oid = pointer[1].decode() + content = ( + storage / "objects" / oid[:2] / oid[2:4] / oid + ).read_bytes() + except (OSError, subprocess.SubprocessError) as error: + raise ValueError( + f"could not retrieve LFS bytes at {relative} from {tag} ({new}); check Git LFS access and refresh manually" + ) from error + if not content_matches( + content, incoming + ) or validator.LFS_POINTER_OID.fullmatch(content): + raise ValueError( + f"invalid LFS SHA-256 or size at {relative} from {tag} ({new}); refresh manually" + ) + proposed[path] = content + check_ledger(local_relative, content, incoming, manifest) + continue + if local == base: + proposed[path] = incoming + elif incoming != base and local != incoming: + for name, content in ( + ("local", local), + ("base", base), + ("incoming", incoming), + ): + (repository / name).write_bytes(content) + try: + proposed[path] = git( + repository, "merge-file", "-p", "local", "base", "incoming" + ) + except subprocess.CalledProcessError as error: + raise ValueError( + f"merge conflict at {local_relative}; rebase the local patch manually; candidate untouched" + ) from error + check_ledger( + local_relative, proposed.get(path, local), incoming, manifest + ) + updated_manifest, replacements = re.subn( + rb"(?m)^( commit:[ \t]*)" + old.encode() + rb"([ \t]*\r?)$", + lambda match: match[1] + new.encode() + match[2], + manifest_path.read_bytes(), + ) + if replacements != 1: + raise ValueError( + "cannot safely replace commit field; normalize the manifest manually" + ) + staged = repository / "candidate" + # Validate the complete proposal, including unchanged files and local + # additions, before touching any original bytes. + import shutil + + shutil.copytree(manifest_path.parent, staged) + for path, content in proposed.items(): + (staged / path.relative_to(manifest_path.parent)).write_bytes(content) + staged_manifest = staged / manifest_path.name + staged_manifest.write_bytes(updated_manifest) + if errors := validator.validate_vendor_manifest(staged_manifest): + raise ValueError( + f"invalid proposed snapshot at {tag} ({new}); review metadata manually: " + + "; ".join(errors) + ) + require_clean(manifest_path) + if not dry_run: + for path, content in {**proposed, manifest_path: updated_manifest}.items(): + if path.read_bytes() != content: + path.write_bytes(content) + print( + f"{manifest_path.parent.name}: {tag} {old} -> {new}" + + (" (already current)" if old == new else "") + + (" (dry-run)" if dry_run else "") + ) + + +def main(root: Path, selection: str, *, dry_run: bool = False) -> int: + manifests = sorted( + (root / validator.EXTERNAL_DEPENDENCIES_ROOT).glob("*/UPSTREAM.yaml") + ) + selected = [ + path + for path in manifests + if selection == "all" or path.parent.name == selection + ] + if not selected: + print( + f"ERROR: unknown dependency {selection}; use a manifest directory name", + file=sys.stderr, + ) + return 1 + failed = False + for path in selected: + try: + refresh_one(path, dry_run=dry_run) + except subprocess.SubprocessError as error: + # Classify stderr, never echo it: Git/credential helpers may include + # rewritten URLs, headers, or arbitrary secrets in diagnostics. + detail = (getattr(error, "stderr", None) or b"").lower() + reason = "Git operation failed; check Git access and the configured pin" + if b"couldn't find remote ref" in detail: + reason = "missing remote branch" + elif any( + word in detail + for word in ( + b"authentication", + b"permission denied", + b"could not read username", + b"403", + b"401", + ) + ): + reason = "authentication/access failure; check Git credentials" + elif isinstance(error, subprocess.TimeoutExpired): + reason = "Git operation timed out; check network access" + elif "merge-base" in error.cmd: + reason = ( + "pin missing or outside configured branch ancestry; review manually" + ) + context = "" + manifest = validator.parse_vendor_manifest(path) + upstream = manifest.get("upstream", {}) + if isinstance(upstream, dict) and not validator.validate_upstream( + path, upstream + ): + context = f" in {upstream['repository']} on branch {upstream['branch']} at pin {upstream['commit']}" + print(f"ERROR: {path.parent.name}: {reason}{context}", file=sys.stderr) + failed = True + except (ValueError, OSError) as error: + print(f"ERROR: {path.parent.name}: {error}", file=sys.stderr) + failed = True + return int(failed) diff --git a/bin/tests/test_refresh_workspace_dependencies.py b/bin/tests/test_refresh_workspace_dependencies.py new file mode 100644 index 000000000..187da8278 --- /dev/null +++ b/bin/tests/test_refresh_workspace_dependencies.py @@ -0,0 +1,722 @@ +"""End-to-end refresh tests using real, disposable Git repositories.""" + +from pathlib import Path +import os +import shutil +import subprocess +import sys + +import pytest + + +BIN = Path(__file__).resolve().parents[1] + + +def git(root, *args): + return subprocess.run( + ["git", "-C", str(root), *args], check=True, capture_output=True, text=True + ).stdout.strip() + + +def commit(root): + git(root, "add", ".") + git(root, "commit", "-qm", "fixture") + return git(root, "rev-parse", "HEAD") + + +@pytest.fixture +def fixture(tmp_path): + upstream = tmp_path / "upstream" + workspace = tmp_path / "workspace" + for root in (upstream, workspace): + root.mkdir() + git(root, "init", "-q", "-b", "main") + git(root, "config", "user.email", "fixture@example.org") + git(root, "config", "user.name", "Fixture") + (upstream / "pkg").mkdir() + (upstream / "pkg/file.txt").write_text("one\ntwo\nthree\nfour\nfive\n") + old = commit(upstream) + git(upstream, "tag", "v1.0.0") + dep = workspace / "src/external_dependencies/demo" + dep.mkdir(parents=True) + shutil.copytree(upstream / "pkg", dep / "pkg") + (dep / "UPSTREAM.yaml").write_text( + "# Keep this annotation\nupstream:\n repository: https://github.com/fixture/upstream.git\n" + f" commit: {old}\n branch: main\nvendored_paths:\n - pkg\n" + "pruning_notes: []\nmodified_paths: []\nnotes:\n - fixture annotation\n" + ) + (workspace / "bin").mkdir() + for script in BIN.glob("*.py"): + shutil.copy2(script, workspace / "bin" / script.name) + commit(workspace) + return upstream, workspace, dep + + +def refresh(workspace, *args): + return subprocess.run( + [ + sys.executable, + str(workspace / "bin/validate_workspace_dependencies.py"), + "--refresh-from-upstream", + *args, + ], + capture_output=True, + text=True, + env={ + **os.environ, + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": f"url.{workspace.parent / 'upstream'}.insteadOf", + "GIT_CONFIG_VALUE_0": "https://github.com/fixture/upstream.git", + }, + ) + + +def test_no_downgrade_or_loss_of_fork_commits(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("fork patch\n") + fork = commit(upstream) + (dep / "pkg/file.txt").write_text("fork patch\n") + manifest = dep / "UPSTREAM.yaml" + import re + + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {fork}", manifest.read_text()) + ) + commit(workspace) + before = manifest.read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "no eligible stable tag" in result.stderr + assert manifest.read_bytes() == before + assert (dep / "pkg/file.txt").read_text() == "fork patch\n" + + +def test_preserves_local_edits_and_pruning_with_snapshot_mapping(fixture): + upstream, workspace, dep = fixture + # Move the upstream root under a local snapshot prefix. + (dep / "nested").mkdir() + shutil.move(dep / "pkg", dep / "nested/pkg") + manifest = dep / "UPSTREAM.yaml" + manifest.write_text( + manifest.read_text() + .replace("vendored_paths:", "snapshot_path: nested\nvendored_paths:") + .replace(" - pkg", " - nested/pkg") + .replace("modified_paths: []", "modified_paths:\n - nested/pkg/file.txt") + ) + (dep / "nested/pkg/file.txt").write_text("one\ntwo\nthree\nfour\nlocal\n") + commit(workspace) + (upstream / "pkg/file.txt").write_text("upstream\ntwo\nthree\nfour\nfive\n") + new = commit(upstream) + git(upstream, "tag", "1.2.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert ( + dep / "nested/pkg/file.txt" + ).read_text() == "upstream\ntwo\nthree\nfour\nlocal\n" + assert f"commit: {new}" in manifest.read_text() + + +@pytest.mark.parametrize("kind", ["unstaged", "staged", "untracked", "ignored"]) +def test_dirty_target_is_untouched(fixture, kind): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("upstream\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + if kind == "ignored": + (workspace / ".gitignore").write_text("scratch\n") + commit(workspace) + target = dep / ("pkg/file.txt" if kind in ("staged", "unstaged") else "scratch") + target.write_text("unsaved work\n") + if kind == "staged": + git(workspace, "add", ".") + before = (dep / "UPSTREAM.yaml").read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "dirty" in result.stderr + assert target.read_text() == "unsaved work\n" + assert (dep / "UPSTREAM.yaml").read_bytes() == before + + +@pytest.mark.parametrize("kind", ["candidate-link", "upstream-link", "traversal"]) +def test_rejects_unsafe_paths_without_writes(fixture, kind): + upstream, workspace, dep = fixture + outside = workspace.parent / "outside" + outside.write_text("do not touch\n") + if kind == "candidate-link": + (dep / "pkg/file.txt").unlink() + (dep / "pkg/file.txt").symlink_to(outside) + elif kind == "upstream-link": + (upstream / "pkg/file.txt").unlink() + (upstream / "pkg/file.txt").symlink_to(outside) + commit(upstream) + git(upstream, "tag", "v2.0.0") + else: + manifest = dep / "UPSTREAM.yaml" + manifest.write_text(manifest.read_text().replace(" - pkg", " - ../demo/pkg")) + commit(workspace) if git(workspace, "status", "--porcelain") else None + before = (dep / "UPSTREAM.yaml").read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "unsafe" in result.stderr or "non-normalized" in result.stderr + assert outside.read_text() == "do not touch\n" + assert (dep / "UPSTREAM.yaml").read_bytes() == before + + +def test_pruned_files_stay_absent_and_local_additions_survive(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/pruned.txt").write_text("prune me\n") + old = commit(upstream) + manifest = dep / "UPSTREAM.yaml" + import re + + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()).replace( + "modified_paths: []", "modified_paths:\n - pkg/local.txt" + ) + ) + (dep / "pkg/local.txt").write_text("local addition\n") + commit(workspace) + (upstream / "pkg/pruned.txt").write_text("still pruned\n") + (upstream / "pkg/file.txt").write_text("new\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert not (dep / "pkg/pruned.txt").exists() + assert (dep / "pkg/local.txt").read_text() == "local addition\n" + assert (dep / "pkg/file.txt").read_text() == "new\n" + + +@pytest.mark.parametrize("change", ["addition", "deletion", "mode"]) +def test_ambiguous_inventory_changes_require_manual_review(fixture, change): + upstream, workspace, dep = fixture + if change == "addition": + (upstream / "pkg/new.txt").write_text("should this be pruned?\n") + elif change == "deletion": + (upstream / "pkg/file.txt").unlink() + else: + (upstream / "pkg/file.txt").chmod(0o755) + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = (dep / "UPSTREAM.yaml").read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "inventory" in result.stderr and "manual" in result.stderr + assert "v2.0.0" in result.stderr + assert git(upstream, "rev-parse", "HEAD") in result.stderr + assert ("pkg/new.txt" if change == "addition" else "pkg/file.txt") in result.stderr + assert (dep / "UPSTREAM.yaml").read_bytes() == before + + +@pytest.mark.parametrize("changed", [False, True]) +def test_upstream_lfs_pointer_never_replaces_local_asset_bytes(fixture, changed): + import hashlib + + upstream, workspace, dep = fixture + payload = b"real mesh bytes\x00\xff" + + def pointer(content): + return f"version https://git-lfs.github.com/spec/v1\noid sha256:{hashlib.sha256(content).hexdigest()}\nsize {len(content)}\n" + + (upstream / "pkg/mesh.bin").write_text(pointer(payload)) + # Commit pointer literally; no filter is installed in this fixture upstream. + old = commit(upstream) + (dep / "pkg/mesh.bin").write_bytes(payload) + manifest = dep / "UPSTREAM.yaml" + import re + + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) + ) + commit(workspace) + if changed: + (upstream / "pkg/mesh.bin").write_text(pointer(b"new mesh")) + (upstream / "pkg/file.txt").write_text("new\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "demo") + if changed: + assert result.returncode == 1 + assert "LFS" in result.stderr and "manual" in result.stderr + else: + assert result.returncode == 0, result.stderr + assert (dep / "pkg/mesh.bin").read_bytes() == payload + + +@pytest.mark.parametrize("absorbed", [False, True]) +def test_invalid_modification_ledger_leaves_candidate_untouched(fixture, absorbed): + upstream, workspace, dep = fixture + (dep / "pkg/file.txt").write_text("one\ntwo\nthree\nfour\nlocal\n") + manifest = dep / "UPSTREAM.yaml" + if absorbed: + manifest.write_text( + manifest.read_text().replace( + "modified_paths: []", "modified_paths:\n - pkg/file.txt" + ) + ) + (upstream / "pkg/file.txt").write_bytes((dep / "pkg/file.txt").read_bytes()) + else: + (upstream / "pkg/file.txt").write_text("new\ntwo\nthree\nfour\nfive\n") + commit(workspace) + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = (dep / "pkg/file.txt").read_bytes(), manifest.read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "modified_paths" in result.stderr + assert before == ((dep / "pkg/file.txt").read_bytes(), manifest.read_bytes()) + + +def test_preview_all_then_targeted_apply(fixture): + upstream, workspace, dep = fixture + other = dep.parent / "other" + shutil.copytree(dep, other) + commit(workspace) + (upstream / "pkg/file.txt").write_text("new\n") + commit(upstream) + git(upstream, "tag", "v1.10.0") + before = (dep / "UPSTREAM.yaml").read_bytes() + preview = refresh(workspace, "all", "--dry-run") + assert preview.returncode == 0, preview.stderr + assert "demo:" in preview.stdout and "other:" in preview.stdout + assert "dry-run" in preview.stdout + assert (dep / "UPSTREAM.yaml").read_bytes() == before + assert (other / "UPSTREAM.yaml").read_bytes() == before + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert (dep / "pkg/file.txt").read_text() == "new\n" + assert (other / "UPSTREAM.yaml").read_bytes() == before + assert refresh(workspace, "missing").returncode == 1 + + +def test_numeric_version_never_decreases_after_untagged_pin(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("pinned\n") + old = commit(upstream) + (dep / "pkg/file.txt").write_text("pinned\n") + manifest = dep / "UPSTREAM.yaml" + import re + + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) + ) + commit(workspace) + (upstream / "pkg/file.txt").write_text("lower version\n") + commit(upstream) + git(upstream, "tag", "v0.9.0") + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "no eligible stable tag" in result.stderr + assert (dep / "pkg/file.txt").read_text() == "pinned\n" + + +def test_conflict_is_actionable_and_entire_dependency_untouched(fixture): + upstream, workspace, dep = fixture + (dep / "pkg/file.txt").write_text("local\n") + manifest = dep / "UPSTREAM.yaml" + manifest.write_text( + manifest.read_text().replace( + "modified_paths: []", "modified_paths:\n - pkg/file.txt" + ) + ) + commit(workspace) + (upstream / "pkg/file.txt").write_text("upstream\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = git(workspace, "status", "--porcelain") + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "conflict" in result.stderr and "pkg/file.txt" in result.stderr + assert "manual" in result.stderr + assert git(workspace, "status", "--porcelain") == before + + +def test_highest_numeric_stable_branch_tag_not_prerelease_or_other_branch(fixture): + upstream, workspace, dep = fixture + for tag in ("v1.9.0", "v1.10.0", "v20.0.0-rc1", "release-30.0.0"): + (upstream / "pkg/file.txt").write_text(tag + "\n") + commit(upstream) + git(upstream, "tag", "-a", tag, "-m", tag) + git(upstream, "checkout", "-qb", "other") + (upstream / "pkg/file.txt").write_text("other branch\n") + commit(upstream) + git(upstream, "tag", "v99.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert (dep / "pkg/file.txt").read_text() == "v1.10.0\n" + + +def test_no_tags_never_uses_head(fixture): + upstream, workspace, dep = fixture + git(upstream, "tag", "-d", "v1.0.0") + result = refresh(workspace, "all") + assert result.returncode == 1 + assert "no eligible stable tag" in result.stderr + assert "branch main" in result.stderr + assert git(upstream, "rev-parse", "HEAD") in result.stderr + assert "https://github.com/fixture/upstream.git" in result.stderr + assert not git(workspace, "status", "--porcelain") + + +def test_duplicate_version_tags_with_different_commits_are_ambiguous(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("first\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + (upstream / "pkg/file.txt").write_text("second\n") + commit(upstream) + git(upstream, "tag", "2.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "ambiguous" in result.stderr + assert not git(workspace, "status", "--porcelain") + + +@pytest.mark.parametrize("kind", ["outside", "apache"]) +def test_snapshot_policy_errors_block_refresh(fixture, kind): + upstream, workspace, dep = fixture + if kind == "outside": + (dep / "stray.txt").write_text("unaccounted\n") + else: + (dep / "pkg/LICENSE").write_text("Apache License\n") + (upstream / "pkg/LICENSE").write_text("Apache License\n") + old = commit(upstream) + manifest = dep / "UPSTREAM.yaml" + import re + + manifest.write_text( + re.sub( + r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text() + ).replace( + "modified_paths: []", + "modified_paths:\n - pkg/file.txt\napache_paths:\n - pkg", + ) + ) + (dep / "pkg/file.txt").write_text( + "one\ntwo\nthree\nfour\nlocal without notice\n" + ) + commit(workspace) + (upstream / "pkg/file.txt").write_text("updated\ntwo\nthree\nfour\nfive\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert ( + "outside vendored_paths" if kind == "outside" else "notice" + ) in result.stderr + assert not git(workspace, "status", "--porcelain") + + +def test_only_commit_field_changes_preserving_crlf_and_note_text(fixture): + upstream, workspace, dep = fixture + manifest = dep / "UPSTREAM.yaml" + old = git(upstream, "rev-parse", "HEAD") + content = ( + manifest.read_text().replace(f" commit: {old}", f" commit: {old} ") + + f" - Historical commit: {old}\n" + ) + original = content.replace("\n", "\r\n").encode() + manifest.write_bytes(original) + commit(workspace) + (upstream / "pkg/file.txt").write_text("new\n") + new = commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert manifest.read_bytes() == original.replace( + f"commit: {old}".encode(), f"commit: {new}".encode() + ) + + +def test_all_apply_and_verify_against_real_upstream(fixture): + upstream, workspace, dep = fixture + other = dep.parent / "other" + shutil.copytree(dep, other) + commit(workspace) + (upstream / "pkg/file.txt").write_text("latest stable\n") + new = commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "all") + assert result.returncode == 0, result.stderr + for target in (dep, other): + assert f"commit: {new}" in (target / "UPSTREAM.yaml").read_text() + # Exercise the existing read-only snapshot verifier after actual apply. + checked = subprocess.run( + [ + sys.executable, + "-B", + "-c", + "import sys; from pathlib import Path; " + "sys.path.insert(0, 'bin'); import validate_workspace_dependencies as v; " + f"errors = v.validate_upstream_snapshot(Path({str(target / 'UPSTREAM.yaml')!r}), Path({str(upstream)!r})); " + "print(errors); sys.exit(bool(errors))", + ], + cwd=workspace, + text=True, + capture_output=True, + ) + assert checked.returncode == 0, checked.stdout + checked.stderr + # Up-to-date invocation after the human commits is a no-op. + commit(workspace) + assert refresh(workspace, "all").returncode == 0 + assert not git(workspace, "status", "--porcelain") + + +def test_edits_made_during_fetch_are_not_overwritten(fixture, monkeypatch): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("new\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + wrappers = workspace.parent / "wrappers" + wrappers.mkdir() + # Real Git still runs. Simulate a human edit while the fetch is in progress. + wrapper = wrappers / "git" + wrapper.write_text( + f"#!{sys.executable}\nimport subprocess, sys\nfrom pathlib import Path\n" + f"result = subprocess.run([{shutil.which('git')!r}, *sys.argv[1:]])\n" + "if 'fetch' in sys.argv:\n" + f" Path({str(dep / 'pkg/file.txt')!r}).write_text('edited during fetch\\n')\n" + "sys.exit(result.returncode)\n" + ) + wrapper.chmod(0o755) + monkeypatch.setenv("PATH", str(wrappers) + os.pathsep + os.environ["PATH"]) + before = (dep / "UPSTREAM.yaml").read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "dirty" in result.stderr + assert (dep / "pkg/file.txt").read_text() == "edited during fetch\n" + assert (dep / "UPSTREAM.yaml").read_bytes() == before + + +@pytest.mark.parametrize("change", ["addition", "deletion", "mode"]) +def test_inventory_changes_in_absent_old_subtree_are_ignored(fixture, change): + import re + + upstream, workspace, dep = fixture + docs = upstream / "pkg/docs" + docs.mkdir() + (docs / "old.txt").write_text("omitted\n") + old = commit(upstream) + manifest = dep / "UPSTREAM.yaml" + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) + ) + commit(workspace) + if change == "addition": + (docs / "new.txt").write_text("also omitted\n") + elif change == "deletion": + (docs / "old.txt").unlink() + else: + (docs / "old.txt").chmod(0o755) + (upstream / "pkg/file.txt").write_text("updated\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert (dep / "pkg/file.txt").read_text() == "updated\n" + assert not (dep / "pkg/docs").exists() + + +@pytest.mark.parametrize("dry_run", [False, True]) +def test_incoming_license_requires_metadata_before_any_write(fixture, dry_run): + import re + + upstream, workspace, dep = fixture + (upstream / "pkg/LICENSE").write_text("BSD-3-Clause\n") + old = commit(upstream) + shutil.copy2(upstream / "pkg/LICENSE", dep / "pkg/LICENSE") + manifest = dep / "UPSTREAM.yaml" + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) + ) + commit(workspace) + (upstream / "pkg/LICENSE").write_text("Apache License\n") + (upstream / "pkg/file.txt").write_text("updated\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = { + p: (p.read_bytes(), p.stat().st_mtime_ns) for p in dep.rglob("*") if p.is_file() + } + result = refresh(workspace, "demo", *(["--dry-run"] if dry_run else [])) + assert result.returncode == 1 + assert "apache_paths" in result.stderr and "manual" in result.stderr + assert before == {p: (p.read_bytes(), p.stat().st_mtime_ns) for p in before} + + +@pytest.mark.parametrize("dry_run", [False, True]) +@pytest.mark.parametrize("failure", [None, "sha", "size", "missing", "local-patch"]) +def test_real_lfs_store_refresh(fixture, failure, dry_run): + import hashlib + import re + + upstream, workspace, dep = fixture + git(upstream, "lfs", "install", "--local") + git(upstream, "lfs", "track", "*.bin") + payload = b"old mesh\x00\xff" + incoming = b"new mesh\x00\xfe" + asset = upstream / "pkg/mesh.bin" + asset.write_bytes(payload) + old = commit(upstream) + (dep / "pkg/mesh.bin").write_bytes(payload) + manifest = dep / "UPSTREAM.yaml" + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) + ) + if failure == "local-patch": + (dep / "pkg/mesh.bin").write_bytes(b"local binary patch\x00") + manifest.write_text( + manifest.read_text().replace( + "modified_paths: []", "modified_paths:\n - pkg/mesh.bin" + ) + ) + commit(workspace) + asset.write_bytes(incoming) + commit(upstream) + oid = hashlib.sha256(incoming).hexdigest() + obj = upstream / ".git/lfs/objects" / oid[:2] / oid[2:4] / oid + assert obj.read_bytes() == incoming + if failure == "sha": + obj.write_bytes(b"x" * len(incoming)) + elif failure == "missing": + obj.unlink() + elif failure == "size": + pointer = git(upstream, "show", "HEAD:pkg/mesh.bin") + "\n" + # Commit a deliberately invalid size without the clean filter repairing it. + (upstream / ".gitattributes").write_text("") + asset.write_text( + pointer.replace(f"size {len(incoming)}", f"size {len(incoming) + 1}") + ) + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = {p: p.read_bytes() for p in dep.rglob("*") if p.is_file()} + result = refresh(workspace, "demo", *(["--dry-run"] if dry_run else [])) + if failure: + assert result.returncode == 1 + assert "LFS" in result.stderr and "pkg/mesh.bin" in result.stderr + assert before == {p: p.read_bytes() for p in before} + else: + assert result.returncode == 0, result.stderr + if dry_run: + assert before == {p: p.read_bytes() for p in before} + else: + assert (dep / "pkg/mesh.bin").read_bytes() == incoming + + +@pytest.mark.parametrize("advance", [False, True]) +def test_unchanged_files_keep_mtimes(fixture, advance): + upstream, workspace, dep = fixture + if advance: + (upstream / "outside.txt").write_text("not retained\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + paths = [dep / "pkg/file.txt"] + ([] if advance else [dep / "UPSTREAM.yaml"]) + before = {p: p.stat().st_mtime_ns for p in paths} + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert before == {p: p.stat().st_mtime_ns for p in paths} + if not advance: + assert "already current" in result.stdout + + +def test_clean_crlf_checkout_has_honest_remedy(fixture): + upstream, workspace, dep = fixture + git(workspace, "config", "core.autocrlf", "true") + target = dep / "pkg/file.txt" + target.unlink() + git(workspace, "checkout", "--", str(target)) + assert b"\r\n" in target.read_bytes() + assert not git(workspace, "status", "--porcelain") + before = target.read_bytes() + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "CRLF" in result.stderr and "pkg/file.txt" in result.stderr + assert "modified_paths disagrees" not in result.stderr + assert "LF checkout" in result.stderr + assert target.read_bytes() == before + + +def test_inherited_git_locations_do_not_redirect_refresh(fixture, monkeypatch): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("updated\n") + commit(upstream) + git(upstream, "tag", "v2.0.0") + config_before = (workspace / ".git/config").read_bytes() + refs_before = git(workspace, "show-ref") + for key, value in { + "GIT_DIR": str(workspace / ".git"), + "GIT_WORK_TREE": str(workspace), + "GIT_INDEX_FILE": str(workspace / ".git/index"), + "GIT_COMMON_DIR": str(workspace / ".git"), + "GIT_OBJECT_DIRECTORY": str(workspace / ".git/objects"), + "GIT_PREFIX": "src/", + }.items(): + monkeypatch.setenv(key, value) + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert (dep / "pkg/file.txt").read_text() == "updated\n" + assert git(workspace, "show-ref") == refs_before + assert (workspace / ".git/config").read_bytes() == config_before + + +def test_missing_branch_error_is_contextual(fixture): + upstream, workspace, dep = fixture + manifest = dep / "UPSTREAM.yaml" + manifest.write_text(manifest.read_text().replace("branch: main", "branch: absent")) + commit(workspace) + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "missing remote branch" in result.stderr + assert "branch absent" in result.stderr + assert git(upstream, "rev-parse", "HEAD") in result.stderr + assert not git(workspace, "status", "--porcelain") + + +def test_git_error_does_not_echo_credentials(fixture, monkeypatch): + upstream, workspace, dep = fixture + wrappers = workspace.parent / "wrappers" + wrappers.mkdir() + wrapper = wrappers / "git" + wrapper.write_text( + f"#!{sys.executable}\nimport subprocess, sys\n" + "if 'fetch' in sys.argv:\n" + " print('fatal: Authentication failed for https://user:secret-token@github.com/fixture/upstream.git Authorization: Bearer secret-token', file=sys.stderr)\n" + " sys.exit(128)\n" + f"sys.exit(subprocess.run([{shutil.which('git')!r}, *sys.argv[1:]]).returncode)\n" + ) + wrapper.chmod(0o755) + monkeypatch.setenv("PATH", str(wrappers) + os.pathsep + os.environ["PATH"]) + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "authentication/access failure" in result.stderr + assert "secret-token" not in result.stderr + assert "branch main" in result.stderr + + +def test_malformed_incoming_lfs_pointer_is_not_installed(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text( + "version https://git-lfs.github.com/spec/v1\noid sha256:invalid\nsize 2\n" + ) + commit(upstream) + git(upstream, "tag", "v2.0.0") + before = {p: p.read_bytes() for p in dep.rglob("*") if p.is_file()} + result = refresh(workspace, "demo") + assert result.returncode == 1 + assert "invalid LFS pointer" in result.stderr + assert "pkg/file.txt" in result.stderr + assert before == {p: p.read_bytes() for p in before} + + +def test_refresh_applies_tag_and_preserves_manifest_annotations(fixture): + upstream, workspace, dep = fixture + (upstream / "pkg/file.txt").write_text("updated\ntwo\nthree\nfour\nfive\n") + new = commit(upstream) + git(upstream, "tag", "v1.1.0") + result = refresh(workspace, "demo") + assert result.returncode == 0, result.stderr + assert (dep / "pkg/file.txt").read_bytes() == ( + upstream / "pkg/file.txt" + ).read_bytes() + assert f"commit: {new}" in (dep / "UPSTREAM.yaml").read_text() + assert "# Keep this annotation" in (dep / "UPSTREAM.yaml").read_text() + assert " - fixture annotation" in (dep / "UPSTREAM.yaml").read_text() diff --git a/bin/validate_workspace_dependencies.py b/bin/validate_workspace_dependencies.py index 18e77a34f..b800dbf55 100644 --- a/bin/validate_workspace_dependencies.py +++ b/bin/validate_workspace_dependencies.py @@ -1313,5 +1313,32 @@ def main(*, verify_upstream: bool = False) -> int: action="store_true", help="fetch pinned upstream branches and verify the modification ledgers", ) + argument_parser.add_argument( + "--refresh-from-upstream", + metavar="all|DEPENDENCY", + help="refresh local vendored snapshots from stable upstream tags (never pushes)", + ) + argument_parser.add_argument( + "--dry-run", + action="store_true", + help="preview a refresh without changing workspace files", + ) arguments = argument_parser.parse_args() + if arguments.dry_run and not arguments.refresh_from_upstream: + argument_parser.error("--dry-run requires --refresh-from-upstream") + if arguments.refresh_from_upstream: + if arguments.verify_upstream: + argument_parser.error( + "choose either --verify-upstream or --refresh-from-upstream" + ) + sys.dont_write_bytecode = True + from refresh_workspace_dependencies import main as refresh_main + + sys.exit( + refresh_main( + REPOSITORY_ROOT, + arguments.refresh_from_upstream, + dry_run=arguments.dry_run, + ) + ) sys.exit(main(verify_upstream=arguments.verify_upstream)) From 8edfeb6c4c6a9c975c3232f67e4554ac3e4eced1 Mon Sep 17 00:00:00 2001 From: "picknik-johnny-5-bot[bot]" <311204565+picknik-johnny-5-bot[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:36:17 +0000 Subject: [PATCH 2/4] fix(deps): identify conflicting release tags --- bin/refresh_workspace_dependencies.py | 10 +++++++--- bin/tests/test_refresh_workspace_dependencies.py | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/bin/refresh_workspace_dependencies.py b/bin/refresh_workspace_dependencies.py index 7790ac831..0f1d88800 100644 --- a/bin/refresh_workspace_dependencies.py +++ b/bin/refresh_workspace_dependencies.py @@ -165,16 +165,20 @@ def refresh_one(manifest_path: Path, *, dry_run: bool = False) -> None: f"containing pin {old}; review the configured branch and refresh manually" ) version, tag = max(versions) - commits = { - git(repository, "rev-parse", f"refs/tags/{name}^{{commit}}") + tag_commits = { + name: git(repository, "rev-parse", f"refs/tags/{name}^{{commit}}") .decode() .strip() for candidate_version, name in versions if candidate_version == version } + commits = set(tag_commits.values()) if len(commits) != 1: + choices = ", ".join( + f"{name}={sha}" for name, sha in sorted(tag_commits.items()) + ) raise ValueError( - "ambiguous stable version tags identify different commits; select and refresh manually" + f"ambiguous stable version tags: {choices}; select and refresh manually" ) new = commits.pop() require_clean(manifest_path) diff --git a/bin/tests/test_refresh_workspace_dependencies.py b/bin/tests/test_refresh_workspace_dependencies.py index 187da8278..9454a97eb 100644 --- a/bin/tests/test_refresh_workspace_dependencies.py +++ b/bin/tests/test_refresh_workspace_dependencies.py @@ -369,6 +369,8 @@ def test_duplicate_version_tags_with_different_commits_are_ambiguous(fixture): result = refresh(workspace, "demo") assert result.returncode == 1 assert "ambiguous" in result.stderr + for tag in ("v2.0.0", "2.0.0"): + assert f"{tag}={git(upstream, 'rev-parse', tag)}" in result.stderr assert not git(workspace, "status", "--porcelain") From c3d25348f05a5f25c5561f4f02688b6ff19fd548 Mon Sep 17 00:00:00 2001 From: "picknik-johnny-5-bot[bot]" <311204565+picknik-johnny-5-bot[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:14:06 +0000 Subject: [PATCH 3/4] refactor(deps): simplify refresh checks and instructions --- README.md | 37 ++++--------- bin/refresh_workspace_dependencies.py | 11 ++-- .../test_refresh_workspace_dependencies.py | 54 ++++++------------- 3 files changed, 31 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 528f01630..c4386c8c8 100644 --- a/README.md +++ b/README.md @@ -46,9 +46,9 @@ The hardware-only `kinova_gen3_site_config` and `picknik_ur_site_config` configu ## Updating vendored dependencies -Each `UPSTREAM.yaml` under `src/external_dependencies` records the upstream repository, branch, exact commit, retained paths, and local modifications. Refresh these snapshots explicitly from the repository root. You need Python 3.10+, Git, Git LFS, network access to the configured repositories, and temporary disk space for upstream Git history, fetched LFS assets, and a staged dependency copy. Commit or stash changes in the dependencies you select first, including untracked and ignored files. Run `git lfs pull` if your checkout still contains LFS pointers instead of asset bytes. +Run from the repository root with Python 3.10+, Git, Git LFS, network access, and space for temporary upstream copies. Selected dependencies must have no uncommitted, untracked, or ignored files. Run `git lfs pull` first if assets are still LFS pointers. -Preview all dependencies, then apply only the one you want: +Preview all dependencies, then refresh one: ```bash python3 bin/validate_workspace_dependencies.py --refresh-from-upstream all --dry-run @@ -56,41 +56,22 @@ python3 bin/validate_workspace_dependencies.py --refresh-from-upstream feetech_r python3 bin/validate_workspace_dependencies.py --refresh-from-upstream feetech_ros2_driver ``` -`feetech_ros2_driver` is the **dependency directory name** under `src/external_dependencies`, not a robot config or its dependency graph. The example may report `already current`; it is not a promise that a newer eligible tag exists. To apply every eligible refresh instead: +Use a directory name under `src/external_dependencies`, not a robot config. Replace it with `all` to refresh every eligible dependency. `--dry-run` checks without writing; apply fetches tags again, so check the reported tag and commit. These commands never commit, push, create a PR, or update optional ML submodules. -```bash -python3 bin/validate_workspace_dependencies.py --refresh-from-upstream all -``` - -These commands fetch upstream data and change only local vendored files and the manifest's `commit` field. They never commit, push, create a PR, or update the optional ML submodules. `--dry-run` performs the same selection and merge checks without writing the snapshot. A later apply fetches tags again, so review the reported tag and commit. - -### Which tag is selected? - -The refresher selects the highest numeric `MAJOR.MINOR.PATCH` tag, optionally prefixed with `v`, whose commit is both reachable from the configured branch and contains the current pinned commit in its ancestry. Annotated and lightweight tags work. Prereleases, build suffixes, and other naming conventions are excluded. The version cannot be lower than the highest stable tag reachable from the current pin. Equal versions with different eligible commits are ambiguous and require manual selection. - -This is deliberately stricter than choosing the newest tag by date or following branch HEAD. In particular, a tag predating PickNik fork commits cannot replace those patches. A pin already at the selected tag is unchanged. If there is no eligible tag, the dependency is left untouched with an error explaining that it needs manual review. There is **no branch-HEAD fallback**. Forks such as `main-picknik` or `ros2-fix-deps` may have no qualifying release. - -### Local patches, pruning, and manual cases - -For an unchanged retained-file inventory (excluding demonstrably already-pruned subtrees), the refresher uses Git's three-way text merge with the old upstream pin as the base. It preserves committed local additions and existing pruned files, honors `snapshot_path`, and keeps manifest comments and annotations byte-for-byte except for the commit field. It checks the modification ledger before and after merging, and validates the complete proposed snapshot and manifest in temporary staging before writing. Incoming license changes that need new metadata stop the refresh for manual review. - -The manifest's `pruning_notes` are prose, not executable selection rules. Changes wholly inside an old upstream directory that is entirely absent locally stay pruned. Other added, deleted, renamed, or mode/type-changed upstream paths inside a retained boundary require a **manual refresh**, rather than guessing which new files to include or silently dropping files. Other manual cases include merge conflicts, symlinks, divergent local binary patches, unsupported LFS-to-ordinary-file transitions, and patches absorbed upstream that need a `modified_paths` correction. Changed upstream LFS assets can update unmodified local assets: Git LFS fetches actual bytes into temporary storage, and the refresher verifies their SHA-256 and size against the selected pointer. Missing or invalid objects stop the dependency; pointers are never installed. Unchanged bytes are not rewritten. - -Clean CRLF checkout conversion is not normalized automatically. Use an LF checkout after reviewing `core.autocrlf` and text/EOL attributes, or refresh manually; do not record checkout conversion as a local patch. +The selected release is the highest stable `MAJOR.MINOR.PATCH` tag (optional `v` prefix) reachable from the branch in `UPSTREAM.yaml` and containing the current pin. Prereleases, build suffixes, downgrades, and ambiguous versions are rejected. There is no branch-HEAD fallback: forks without an eligible tag need manual review. -A selection, merge, LFS retrieval, or proposed-validation error leaves that dependency untouched. Writes are not crash-atomic: an interruption or write-time disk failure can leave a partial update. In `all` mode, independent successful dependencies can still be refreshed; the command exits nonzero if any dependency fails. Inspect each reported result. For a manual refresh, check out the chosen upstream commit separately, preserve licenses and notices, reapply local patches and pruning, and update the manifest's commit and path declarations. Do not change branches merely to bypass a missing tag or discard fork patches. +Local patches and pruning are preserved where they can be merged safely. Conflicts, uncertain file selection, unsupported binary changes, and required license or manifest corrections stop that dependency before writing. Follow the reported diagnostic rather than bypassing it. In `all` mode, other dependencies can still succeed; any failure returns a nonzero exit code. An interruption during writing can leave a partial update. -### Review and publish the local changes +Review and validate the changes: ```bash -git diff --stat git diff -- src/external_dependencies python3 bin/validate_workspace_dependencies.py # offline structure check -python3 bin/validate_workspace_dependencies.py --verify-upstream # read-only comparison with each pinned upstream commit +python3 bin/validate_workspace_dependencies.py --verify-upstream # read-only network comparison git diff --check ``` -Review release notes, license changes, and any provenance notes that mention an older release, then build and test every robot config that consumes the changed packages. The refresher does not prove runtime compatibility. After review and validation, commit and push through your normal PR workflow, for example for a single dependency: +Review release notes and licenses, update stale provenance notes, and test every robot config that uses the changed packages. Then commit and push, for example: ```bash git add src/external_dependencies/feetech_ros2_driver @@ -98,4 +79,4 @@ git commit -m "Refresh vendored Feetech driver dependency" git push ``` -`--verify-upstream` needs network access but does not refresh files. CI checks structure only; it does not compare vendored contents, schedule refreshes, or create upstream-verification issues. The optional ML model submodules can be advanced independently when their demonstration Objectives need a newer model package. +CI checks structure only. Upstream comparison and refresh are user-triggered. diff --git a/bin/refresh_workspace_dependencies.py b/bin/refresh_workspace_dependencies.py index 0f1d88800..a976da0e4 100644 --- a/bin/refresh_workspace_dependencies.py +++ b/bin/refresh_workspace_dependencies.py @@ -48,14 +48,13 @@ def content_matches(local: bytes, upstream: bytes) -> bool: def check_ledger(path: Path, local: bytes, upstream: bytes, manifest: dict) -> None: - if (not content_matches(local, upstream)) != ( - path.as_posix() in manifest.get("modified_paths", []) - ): + modified = not content_matches(local, upstream) + if modified != (path.as_posix() in manifest.get("modified_paths", [])): raise ValueError( f"modified_paths disagrees at {path}; reconcile the ledger manually (including absorbed patches)" ) if ( - not content_matches(local, upstream) + modified and validator.path_is_declared( path, {Path(p) for p in manifest.get("apache_paths", [])} ) @@ -199,10 +198,10 @@ def refresh_one(manifest_path: Path, *, dry_run: bool = False) -> None: inventories.append(inventory) # Only directories present upstream at the old pin and entirely absent # locally prove prior subtree pruning. New siblings remain ambiguous. + old_ancestors = {parent for path in inventories[0] for parent in path.parents} pruned = { parent - for path in inventories[0] - for parent in path.parents + for parent in old_ancestors if validator.path_is_declared(parent, boundaries) and not (manifest_path.parent / snapshot / parent).exists() } diff --git a/bin/tests/test_refresh_workspace_dependencies.py b/bin/tests/test_refresh_workspace_dependencies.py index 9454a97eb..cd2ec6946 100644 --- a/bin/tests/test_refresh_workspace_dependencies.py +++ b/bin/tests/test_refresh_workspace_dependencies.py @@ -2,6 +2,7 @@ from pathlib import Path import os +import re import shutil import subprocess import sys @@ -24,6 +25,12 @@ def commit(root): return git(root, "rev-parse", "HEAD") +def set_pin(manifest, sha): + manifest.write_text( + re.sub(r"commit: [0-9a-f]+", f"commit: {sha}", manifest.read_text()) + ) + + @pytest.fixture def fixture(tmp_path): upstream = tmp_path / "upstream" @@ -77,11 +84,7 @@ def test_no_downgrade_or_loss_of_fork_commits(fixture): fork = commit(upstream) (dep / "pkg/file.txt").write_text("fork patch\n") manifest = dep / "UPSTREAM.yaml" - import re - - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {fork}", manifest.read_text()) - ) + set_pin(manifest, fork) commit(workspace) before = manifest.read_bytes() result = refresh(workspace, "demo") @@ -167,10 +170,9 @@ def test_pruned_files_stay_absent_and_local_additions_survive(fixture): (upstream / "pkg/pruned.txt").write_text("prune me\n") old = commit(upstream) manifest = dep / "UPSTREAM.yaml" - import re - + set_pin(manifest, old) manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()).replace( + manifest.read_text().replace( "modified_paths: []", "modified_paths:\n - pkg/local.txt" ) ) @@ -223,11 +225,7 @@ def pointer(content): old = commit(upstream) (dep / "pkg/mesh.bin").write_bytes(payload) manifest = dep / "UPSTREAM.yaml" - import re - - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) - ) + set_pin(manifest, old) commit(workspace) if changed: (upstream / "pkg/mesh.bin").write_text(pointer(b"new mesh")) @@ -295,11 +293,7 @@ def test_numeric_version_never_decreases_after_untagged_pin(fixture): old = commit(upstream) (dep / "pkg/file.txt").write_text("pinned\n") manifest = dep / "UPSTREAM.yaml" - import re - - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) - ) + set_pin(manifest, old) commit(workspace) (upstream / "pkg/file.txt").write_text("lower version\n") commit(upstream) @@ -384,12 +378,9 @@ def test_snapshot_policy_errors_block_refresh(fixture, kind): (upstream / "pkg/LICENSE").write_text("Apache License\n") old = commit(upstream) manifest = dep / "UPSTREAM.yaml" - import re - + set_pin(manifest, old) manifest.write_text( - re.sub( - r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text() - ).replace( + manifest.read_text().replace( "modified_paths: []", "modified_paths:\n - pkg/file.txt\napache_paths:\n - pkg", ) @@ -492,17 +483,13 @@ def test_edits_made_during_fetch_are_not_overwritten(fixture, monkeypatch): @pytest.mark.parametrize("change", ["addition", "deletion", "mode"]) def test_inventory_changes_in_absent_old_subtree_are_ignored(fixture, change): - import re - upstream, workspace, dep = fixture docs = upstream / "pkg/docs" docs.mkdir() (docs / "old.txt").write_text("omitted\n") old = commit(upstream) manifest = dep / "UPSTREAM.yaml" - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) - ) + set_pin(manifest, old) commit(workspace) if change == "addition": (docs / "new.txt").write_text("also omitted\n") @@ -521,16 +508,12 @@ def test_inventory_changes_in_absent_old_subtree_are_ignored(fixture, change): @pytest.mark.parametrize("dry_run", [False, True]) def test_incoming_license_requires_metadata_before_any_write(fixture, dry_run): - import re - upstream, workspace, dep = fixture (upstream / "pkg/LICENSE").write_text("BSD-3-Clause\n") old = commit(upstream) shutil.copy2(upstream / "pkg/LICENSE", dep / "pkg/LICENSE") manifest = dep / "UPSTREAM.yaml" - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) - ) + set_pin(manifest, old) commit(workspace) (upstream / "pkg/LICENSE").write_text("Apache License\n") (upstream / "pkg/file.txt").write_text("updated\n") @@ -549,7 +532,6 @@ def test_incoming_license_requires_metadata_before_any_write(fixture, dry_run): @pytest.mark.parametrize("failure", [None, "sha", "size", "missing", "local-patch"]) def test_real_lfs_store_refresh(fixture, failure, dry_run): import hashlib - import re upstream, workspace, dep = fixture git(upstream, "lfs", "install", "--local") @@ -561,9 +543,7 @@ def test_real_lfs_store_refresh(fixture, failure, dry_run): old = commit(upstream) (dep / "pkg/mesh.bin").write_bytes(payload) manifest = dep / "UPSTREAM.yaml" - manifest.write_text( - re.sub(r"commit: [0-9a-f]+", f"commit: {old}", manifest.read_text()) - ) + set_pin(manifest, old) if failure == "local-patch": (dep / "pkg/mesh.bin").write_bytes(b"local binary patch\x00") manifest.write_text( From 8f11250a7d06a3a4302755bf6fe7534e1af9786f Mon Sep 17 00:00:00 2001 From: "picknik-johnny-5-bot[bot]" <311204565+picknik-johnny-5-bot[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:41:29 +0000 Subject: [PATCH 4/4] docs: preserve manual dependency refresh instructions --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index c4386c8c8..7cffce5f7 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,23 @@ The hardware-only `kinova_gen3_site_config` and `picknik_ur_site_config` configu ## Updating vendored dependencies +Each `UPSTREAM.yaml` under `src/external_dependencies` records the exact upstream commit and retained paths. To refresh one: check the tree out at the new commit, preserve its license files, reapply the pruning described in `pruning_notes`, and validate every config that consumes the package. + +Then update `commit` and the retained-path lists, and check the result: + +```bash +python3 bin/validate_workspace_dependencies.py # structure, runs on every PR +python3 bin/validate_workspace_dependencies.py --verify-upstream # fetches the pinned commit and compares files +``` + +The second command needs network access. Run it manually after re-vendoring. CI checks structure only; it does not compare vendored contents or create upstream-verification issues. + +The optional ML model submodules can be advanced independently when their demonstration Objectives need a newer model package. + +### Optional quick refresh + +For eligible upstream releases, the command below is a quicker alternative to the manual workflow above. + Run from the repository root with Python 3.10+, Git, Git LFS, network access, and space for temporary upstream copies. Selected dependencies must have no uncommitted, untracked, or ignored files. Run `git lfs pull` first if assets are still LFS pointers. Preview all dependencies, then refresh one: