Skip to content

Quote FreeSurfer stub paths in link helper - #950

Merged
m-reuter merged 1 commit into
Deep-MI:devfrom
jdotc1:fix/freesurfer-stub-path
Oct 14, 2026
Merged

m-reuter merged 1 commit into
Deep-MI:devfrom
jdotc1:fix/freesurfer-stub-path

Conversation

@jdotc1

@jdotc1 jdotc1 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Quote the stub destination when writing not-here.sh and setting its executable permission. An installation directory containing spaces previously caused an ambiguous redirect and left the generated links without an executable target.

This two-line change is confined to tools/build/link_fs.sh, which is also bundled with the macOS installer. It changes path handling rather than image processing or architecture selection.

Validation

python3 ../check-fastsurfer-link.py

One synthetic check invokes the actual helper with /bin/bash against an empty temporary FreeSurfer directory containing spaces. It verifies that the stub exists and is executable, that the generated links resolve to it, and that a link responds to the supported version-information option. The regression is included in test/shell/test_link_fs.py; the standalone runner loads that test and supplies a temporary directory.

The check failed against the original helper because the target was not created. It passed after the fix. No FreeSurfer installation, imaging data, package build, or broad suite was used. Repository-wide lint and spelling checks were omitted to keep validation within the explicitly requested focused check.

The earlier helper consolidation in #775 does not quote these remaining expansions.

@m-reuter

Copy link
Copy Markdown
Member

Thanks for your contribution! Both our installations (docker and Mac package) use paths without spaces, so this is only for people running it themselves, but still the fix makes sense and will be merged.

@m-reuter
m-reuter merged commit 4eb1b4b into Deep-MI:dev Oct 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants