fix(aqua): skip in-place link when src and dst alias same inode#10012
Conversation
Greptile SummaryThis PR adds a same-inode/same-canonical-path guard inside
Confidence Score: 5/5The change is a narrowly-scoped, additive guard that only fires when both paths already resolve to the same file — it cannot remove or corrupt data. The guard correctly uses dev+ino on Unix (covering both case-insensitive aliases and hardlinked siblings) and canonicalize on Windows (covering the NTFS case-folding bug). It fires before all three link-creation branches, the fallback on error is conservatively false (proceed rather than silently skip), and the regression test exercises the exact inode-aliasing scenario on Unix CI. No existing code paths are altered outside the early-return. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(aqua): skip in-place link when src a..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request introduces a guard in AquaBackend::create_file_link to prevent overwriting files with self-referential links on case-insensitive filesystems by checking if the source and destination refer to the same disk entry. It includes a new helper function same_disk_entry with platform-specific implementations for Unix and non-Unix systems, along with a corresponding unit test. I have no feedback to provide.
On case-insensitive filesystems like macOS APFS, `link.src` and `link.dst` can differ as strings but resolve to the same on-disk file; the symlink branch then removes the source binary before recreating it, leaving a broken self-referential symlink.
Summary
srcanddstdiffer only in case — like godot'sGodot.app/Contents/MacOS/Godot→godot— resolves both paths to the same on-disk file.AquaBackend::create_file_linkthen removed the source binary and replaced it with a self-referential symlink, leaving the install broken.create_file_link: whendstexists and resolves to the same on-disk entry assrc, return early. The check usesdev+inoon unix andcanonicalizeon other platforms.hard:truehardlink, Windows copy, unix symlink), since each previously calledremove_file(dst)before recreating the link.Reproducer
Before this PR:
godot -> Godot(broken symlink), noGodotbinary.After this PR:
Godotbinary present, siblinggodotsymlink works.Approach
The
!dst.exists()precondition at the call site was removed earlier to supporthard:trueoverwrite semantics, which exposed the case-insensitive aliasing case. Rather than restore that precondition (which would re-break overwrite), the guard lives insidecreate_file_linkand only short-circuits when the two paths demonstrably refer to the same inode/canonical path — a no-op situation either way.fs::canonicalize-based comparison alone isn't enough: it correctly identifies APFS/NTFS case-insensitive aliases but not hardlinked siblings. Usingdev+inoon unix covers both, which keeps the regression test portable on case-sensitive Linux CI.Other backends that create similar links (
conda,github, etc.) retain their own!dst.exists()guards, so they aren't affected by this pattern.Note for affected users
This fix prevents new installs from being corrupted. Existing installs that already have a self-referential symlink (godot, plus any other aqua packages with case-only link renames on macOS) still need a manual repair:
This code was generated with help from Opus 4.7 in Claude Code