Skip to content

Fix the help check and report failures in link_fs.sh - #951

Merged
m-reuter merged 1 commit into
Deep-MI:devfrom
m-reuter:link-fs
Oct 14, 2026
Merged

m-reuter merged 1 commit into
Deep-MI:devfrom
m-reuter:link-fs

Conversation

@m-reuter

Copy link
Copy Markdown
Member

Follow-up to #950, for two older issues in tools/build/link_fs.sh:

  • The help check matched -h anywhere in the arguments, so a FreeSurfer path containing it (e.g. .../fs-hpc) printed the usage and exited with 0 without linking anything. It now matches only the whole arguments -h and --help.
  • A stub that could not be written, or a link that could not be created, went unreported, which left dangling links behind. The script now stops with an error, so the Docker build and the macOS postinstall fail visibly.

The test from #950 now also runs on a path containing -h, and a new test checks that a missing bin directory stops the script with an error.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fixes correctly handle help arguments and report failures, with no blocking issues identified.

0 open findings

What changed in this PR

This follow-up to #950 improves the FreeSurfer link helper’s argument handling and failure reporting.

Changes:

  • Recognize only whole -h and --help arguments.
  • Stop on stub creation, permission, or link failures.
  • Extend regression tests for paths containing -h and missing bin directories.
File Description
tools/​build/​link_fs.sh Match exact help flags and exit on creation failures.
test/​shell/​test_link_fs.py Cover installation paths and stub-write failure reporting.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@m-reuter
m-reuter merged commit 3a71dde into Deep-MI:dev Oct 14, 2026
4 checks passed
@m-reuter
m-reuter deleted the link-fs branch October 14, 2026 22:04
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