Skip to content

install.sh fails with 'mv: cannot stat .dpcli' - #5

Merged
github-actions[bot] merged 3 commits into
mainfrom
improve/install-missing-dpcli
Oct 29, 2026
Merged

github-actions[bot] merged 3 commits into
mainfrom
improve/install-missing-dpcli

Conversation

@zolotarev

@zolotarev zolotarev commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Friction — a user's first install stopped with mv: cannot stat '.dpcli': git tracks .dpcli but the directory is missing. Two causes: a gitlink without a .gitmodules entry, or a vendored copy tracked but deleted from disk; both fell into the vendored-migration branch.

Change — a gitlink is registered in .gitmodules and checked out (re-added if its commit is unknown); a deleted vendored copy is restored from the index before migration. Two tests reproduce both cases.

Budget increase justification — risky.force 21 → 22: git submodule add --force in the fallback path, needed so the add reuses the local module git dir left by the failed submodule update; it never overwrites a working tree (the path is first emptied with rmdir, which fails on non-empty dirs).

…d copy deleted from disk

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zolotarev zolotarev added the feedback Friction reported from real use label Oct 29, 2026
@github-actions

Copy link
Copy Markdown

VERDICT: rework

  • Real friction (first install fails with mv: cannot stat), both causes get a test, VERSION 14 and the CHANGELOG line are in place, English, anonymized. The fix direction is right.
  • install.sh:50-52 — the fallback git rm -q --cached .dpcli; rm -rf .dpcli deletes the existing checkout without a backup. If submodule update fails because the commit is unknown, that directory may hold local commits or edits. The vendored branch moves it to $(git rev-parse --git-dir)/dpcli-vendored-* instead. Do the same here: mv .dpcli "<backup>" instead of rm -rf, and say so in the output.
  • tests/test_install.py — the fallback (unknown commit → re-add with --force) has no test. test_gitlink_without_gitmodules uses HEAD of src, which is always known, so it only covers the happy path. Add a case with a nonexistent sha, or drop the fallback if no real case needs it (deleting beats adding).
  • install.sh:44 — git ls-files -s .dpcli | grep -q '^160000' runs under set -o pipefail. Use git ls-files -s .dpcli | grep -q … only if you're sure about SIGPIPE, or avoid the pipe with [ "$(git ls-files -s .dpcli | cut -c1-6)" = 160000 ]. Output is one line, so low risk. Nit only.
  • Not verified: I could not run the test suite in the review sandbox. CI must be green before merge.

…bmodule add reuses the local module dir)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

VERDICT: rework

  • Real friction (first install dies with mv: cannot stat), two causes, VERSION 14 + CHANGELOG line present, English, anonymized. The earlier rm -rf concern is fixed: rmdir only removes an empty dir, so no user data is lost.
  • tests/test_install.py — the fallback (git rm --cached + rmdir + submodule add --force, install.sh:48-51) still has no test. test_gitlink_without_gitmodules uses HEAD of src, a known commit, so submodule update succeeds and the fallback never runs. This is the only code that justifies raising risky.force 21 → 22. Add a case with a nonexistent sha (the gitlink commit unknown to $REPO) and assert .dpcli/dpcli exists afterwards. If it can't be reproduced, drop the fallback and the budget bump (deleting beats adding).
  • install.sh:48-51 — if .dpcli is non-empty at that point (partial checkout), rmdir is silently skipped and submodule add then fails with a raw git error under set -e. Print a die hint such as "move .dpcli away and rerun" instead.
  • install.sh:41 — the .gitmodules lookup only matches a submodule named .dpcli. If the entry exists under another name (path .dpcli), the new branch adds a duplicate. Minor; consider looking up by path.
  • Not verified: I could not run the test suite in the review sandbox; CI must be green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

VERDICT: accept

  • Real friction (feedback label, reproduced from a first install); both causes covered by tests that fail on the old script (tests/test_install.py:69-94).
  • Safe: --force (install.sh:50) only runs after git rm --cached and rmdir, which fails on a non-empty dir, so no working tree is overwritten. The risky.force 21→22 bump is justified.
  • Process OK: VERSION 14, CHANGELOG line, English, no project data. No new dependencies.
  • Nit: if rmdir leaves a non-empty .dpcli (install.sh:49-50), set -e stops after the gitlink was already removed from the index. A hint such as "move .dpcli away and rerun" would be kinder; not blocking.
  • Nit: git checkout -- .dpcli (install.sh:56) runs only when the whole directory is missing. A partially deleted vendored copy still goes through mv as before; fine for now.
  • Limit: I could not run the test suite in the review sandbox. Verdict is from reading the diff; CI should confirm.

@github-actions github-actions Bot removed the rework label Oct 29, 2026
@github-actions
github-actions Bot merged commit 3b06f41 into main Oct 29, 2026
3 checks passed
@github-actions
github-actions Bot deleted the improve/install-missing-dpcli branch October 29, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback Friction reported from real use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant