Fix: symlink may point to absolute path#799
Conversation
|
The doc build error is a problem inherited from pyrr, and this PR does not have any changes to the doc. (can be ignored) |
There was a problem hiding this comment.
Pull request overview
Fixes symlink/copy behavior so generated stats symlinks remain valid when $subject_dir is mounted at different paths (e.g., Docker/Singularity), and makes softlink_or_copy’s copy fallback handle relative link targets correctly.
Changes:
- Convert an absolute
$asegdkt_vinn_statsfilesymlink target into a relative path before linkingaseg+DKT.stats. - Update
softlink_or_copyto resolve relative link targets correctly when falling back from symlink to copy. - Add/adjust inline documentation around
softlink_or_copybehavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| run_fastsurfer.sh | Computes a relative target path before creating the legacy aseg+DKT.stats link. |
| recon_surf/functions.sh | Improves softlink_or_copy copy fallback for relative symlink targets and updates comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dkuegler
force-pushed
the
fix/asegdkt-statsfile-symlink
branch
from
823c46c to
f8a27ee
Compare
…tats/aseg+DKT.stats was not pointing to the correct file. aseg+DKT.stats is usually a symlink to aseg+DKT.VINN.stats, but the target was previously an absolute path, which would create invalid paths if created inside a docker or singularity image that mounted $subject_dir somewhere else. Another failure "opportunity" was when files were moved. Furthermore, the copy option of softlink_or_copy would fail, if a relative path was used, but the current working directory was not the directory of the link.
dkuegler
force-pushed
the
fix/asegdkt-statsfile-symlink
branch
from
f8a27ee to
027eeaa
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses portability issues around stats-file symlinks/copies when running inside containers or after subject directory moves, by ensuring the legacy aseg+DKT.stats link points correctly and by improving softlink_or_copy handling of relative link targets.
Changes:
- Convert the
aseg+DKT.VINN.statspath to a relative path (when possible) before creating the legacyaseg+DKT.statslink inrun_fastsurfer.sh. - Update
softlink_or_copyto better document semantics and attempt to correctly resolve relative link targets when falling back to copying. - Add a
relative_to()helper (Python-based) to compute a relative path from a link location to its target.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| run_fastsurfer.sh | Computes a relative path for aseg+DKT.VINN.stats before linking to avoid absolute symlinks in containers. |
| recon_surf/functions.sh | Refactors softlink_or_copy and introduces relative_to() to support relative symlink targets and copy fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR fixes an error in run_fastsurfer.sh, where $subject_dir/stats/aseg+DKT.stats was not pointing to the correct file. aseg+DKT.stats is usually a symlink to aseg+DKT.VINN.stats, but the target was previously an absolute path, which would create invalid paths if created inside a docker or singularity image that mounted $subject_dir somewhere else. Another failure "opportunity" was when files were moved.
Furthermore, the copy option of softlink_or_copy would fail, if a relative path was used, but the current working directory was not the directory of the link.