Skip to content

Conversation

@jdtournier
Copy link
Member

@jdtournier jdtournier commented Oct 11, 2023

Recent versions of FSL seem to ship with a full working miniconda environment, including a compiler and associated tools, and the full Qt SDK. This will cause all kinds of conflicts at the configure stage. This commit detects and mention of 'fsl' in any component and the PATH and strips it out, using the same approach as for anaconda.

This has led to problems reported on the forum, e.g.:
https://community.mrtrix.org/t/a-problem-of-dwifslpreproc/6959?u=jdtournier
https://community.mrtrix.org/t/error-linking-qt-application-centos7/6851?u=jdtournier

@jdtournier jdtournier requested a review from a team October 11, 2023 08:34
@jdtournier jdtournier self-assigned this Oct 11, 2023
@Lestropie Lestropie added this to the 3.0.5 updates milestone Sep 16, 2024
@daljit46
Copy link
Member

Is this still required? I believe that the outcome of our discussion was to inform users how to properly setup FSL using their latest scripts (which shouldn't cause the issue this PR addresses).

@Lestropie
Copy link
Member

I'm pretty sure I discovered that FreeSurfer's SetUpFreeSurfer.sh was in fact adding FSL's bin/ directory rather than share/fsl/bin to PATH, which means that this is likely to remain widespread.

@jdtournier
Copy link
Member Author

I can confirm that FreeSurfer does add $FSLDIR/bin to the PATH - happened to me a few weeks ago...

@daljit46
Copy link
Member

Ok, it seems that is the case indeed.
Would it be acceptable just to warn users rather than modifying the PATH ourselves?

@jdtournier
Copy link
Member Author

I must admit I'm not sure what the right thing to do is here... But given that this should only affect building the software, it's now a much smaller problem than it would have been in the past. For building, I think a warning is the only reasonable thing to do, as long as it provides hints as to where to look, i.e.:

  • check your PATH does not include anything that looks like $FSLDIR/bin - only $FSLDIR/share/fsl/bin
  • use which python, which $CXX, etc to double-check which versions are being used, and that you're not using the version supplied with Anaconda, FSL, FreeSurfer or some other project
  • when trying to figure out where these folders are being added to the PATH, bear in mind that FreeSurfer brings in the wrong folder for their version of FSL (link to issue).

It may be best to provide all this as a wiki and link to that in the warning, to be honest...

As for runtime, this can also cause problems as you may end up using a different version of Python from that expected. Can be frustrating when adding missing modules, or upgrading your system-wide version of Python, etc results in exactly the same problematic outcome when running one of our scripts...

@daljit46
Copy link
Member

@MRtrix3/mrtrix3-devs We discussed this on Wednesday with @bjeurissen and @Lestropie and while I think providing a warning would be preferable, I'm happy for this to go ahead as this is only relevant to the old build system and will be discarded in 3.1.0 anyway.

Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

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

Agreed; happy to keep with the philosophy of expedite-solution-for-most-users with the old build system.

@Lestropie Lestropie added this pull request to the merge queue Nov 23, 2024
Merged via the queue into master with commit e8ca0be Nov 23, 2024
@Lestropie Lestropie deleted the strip_fsl_from_path branch November 23, 2024 05:48
@Lestropie Lestropie mentioned this pull request Jan 14, 2025
31 tasks
@Lestropie Lestropie mentioned this pull request Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants