fix(doctor): fallback to environment-honouring Repo constructor for linked worktrees (#1931) - #1952
Conversation
|
✅ Health of changed files: 1.6 (unchanged) 📋 At a glance ✅ Health gate: passed 📌 Before you merge
🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_packages_cli_src_repowise_cli_commands_doctor_cmd_repo_checks_py[".../doctor_cmd/repo_checks.py 🔥"]:::changed
end
f_packages_cli_src_repowise_cli_commands_doctor_cmd___init___py[".../doctor_cmd/__init__.py"]
f_packages_cli_src_repowise_cli_commands_doctor_cmd_repo_checks_py --> f_packages_cli_src_repowise_cli_commands_doctor_cmd___init___py
f_packages_cli_src_repowise_cli_commands_doctor_cmd_command_py[".../doctor_cmd/command.py"]
f_packages_cli_src_repowise_cli_commands_doctor_cmd_repo_checks_py --> f_packages_cli_src_repowise_cli_commands_doctor_cmd_command_py
t_tests_unit_cli_test_agents_cmd_py(["✅ .../cli/test_agents_cmd.py"]):::guard
t_tests_unit_cli_test_agents_cmd_py -.-> f_packages_cli_src_repowise_cli_commands_doctor_cmd_repo_checks_py
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (2 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspot touched (1)
👀 Suggested reviewers @RaghavChamadiya 📊 See the full report for this PR |
|
Thanks @teddiesloco. #1931 is a real bug and you have the mechanism right: in a linked worktree or a container, The fallback as written turns a false negative into a false positive, though, which is a worse trade for a diagnostic command.
The explicit constructor raises, the fallback finds the repo you happen to be standing in, and doctor appends: checks.append(_check("Git repository", True, str(repo_path)))which reports The check needs to establish that the discovered repo is the one at Only fall back when the environment is actually directing you, and confirm the answer points back at the path: except Exception:
if not (os.environ.get("GIT_DIR") or os.environ.get("GIT_WORK_TREE")):
raise
found = gitpython.Repo()
if found.working_tree_dir is None or not Path(repo_path).resolve().is_relative_to(
Path(found.working_tree_dir).resolve()
):
raiseOr resolve the worktree's gitdir yourself and hand it over explicitly, which is what git does when it reads a Either way the Two other things. There is no test. The repo's convention is a test alongside a behaviour change, and this one is easy to pin without a real worktree: a The new comment runs past the line length the rest of the file keeps to. Cosmetic, and ruff is not complaining, so only worth doing while you are in there. Get the cwd case ruled out and add that one test and I will merge it. The underlying fix is the right idea and #1931 deserves to be closed. |
Summary
Fixes #1931.
When running
repowise doctorin a linked git worktree or container environment where the worktree's.gitfile points to a host path that doesn't directly resolve via explicit path, passinggitpython.Repo(repo_path)fails withNot a git repo.This PR:
gitpython.Repo()when the explicit path constructor fails, allowing GitPython to honourGIT_DIR,GIT_COMMON_DIR, andGIT_WORK_TREEenvironment variables.doctoraccurately reports healthy git repository status for worktrees without false positives.Test Plan