Merge pull request #2165 from oesteban/fix/fs-license-canary

FIX: FreeSurfer license manipulation & canary

Author: oesteban

fmriprep/cli/parser.py

@@ -23,6 +23,13 @@ def _path_exists(path, parser):
             raise parser.error(f"Path does not exist: <{path}>.")
         return Path(path).absolute()
 
+    def _is_file(path, parser):
+        """Ensure a given path exists and it is a file."""
+        path = _path_exists(path, parser)
+        if not path.is_file():
+            raise parser.error(f"Path should point to a file (or symlink of file): <{path}>.")
+        return path
+
     def _min_one(value, parser):
         """Ensure an argument is not lower than 1."""
         value = int(value)
@@ -63,6 +70,7 @@ def _bids_filter(value):
         formatter_class=ArgumentDefaultsHelpFormatter,
     )
     PathExists = partial(_path_exists, parser=parser)
+    IsFile = partial(_is_file, parser=parser)
     PositiveInt = partial(_min_one, parser=parser)
 
     # Arguments as specified by BIDS-Apps
@@ -128,7 +136,7 @@ def _bids_filter(value):
         dest="bids_filters",
         action="store",
         type=_bids_filter,
-        metavar="PATH",
+        metavar="FILE",
         help="a JSON file describing custom BIDS input filters using PyBIDS. "
         "For further details, please check out "
         "https://fmriprep.readthedocs.io/en/%s/faq.html#"
@@ -178,8 +186,10 @@ def _bids_filter(value):
     )
     g_perfm.add_argument(
         "--use-plugin",
+        "--nipype-plugin-file",
         action="store",
-        default=None,
+        metavar="FILE",
+        type=IsFile,
         help="nipype plugin configuration file",
     )
     g_perfm.add_argument(
@@ -414,8 +424,8 @@ def _bids_filter(value):
     g_fs = parser.add_argument_group("Specific options for FreeSurfer preprocessing")
     g_fs.add_argument(
         "--fs-license-file",
-        metavar="PATH",
-        type=PathExists,
+        metavar="FILE",
+        type=IsFile,
         help="Path to FreeSurfer license key file. Get it (for free) by registering"
         " at https://surfer.nmr.mgh.harvard.edu/registration.html",
     )
@@ -551,7 +561,6 @@ def parse_args(args=None, namespace=None):
     """Parse args and run further checks on the command line."""
     import logging
     from niworkflows.utils.spaces import Reference, SpatialReferences
-    from niworkflows.utils.misc import check_valid_fs_license
 
     parser = _build_parser()
     opts = parser.parse_args(args, namespace)
@@ -567,15 +576,6 @@ def parse_args(args=None, namespace=None):
     # Retrieve logging level
     build_log = config.loggers.cli
 
-    if not check_valid_fs_license(lic=config.execution.fs_license_file):
-        raise RuntimeError(
-            """\
-ERROR: a valid license file is required for FreeSurfer to run. fMRIPrep looked for an existing \
-license file at several paths, in this order: 1) command line argument ``--fs-license-file``; \
-2) ``$FS_LICENSE`` environment variable; and 3) the ``$FREESURFER_HOME/license.txt`` path. Get it \
-(for free) by registering at https://surfer.nmr.mgh.harvard.edu/registration.html"""
-        )
-
     # Load base plugin_settings from file if --use-plugin
     if opts.use_plugin is not None:
         import yaml

fmriprep/cli/workflow.py

@@ -13,6 +13,7 @@
 def build_workflow(config_file, retval):
     """Create the Nipype Workflow that supports the whole execution graph."""
     from niworkflows.utils.bids import collect_participants, check_pipeline_version
+    from niworkflows.utils.misc import check_valid_fs_license
     from niworkflows.reports import generate_reports
     from .. import config
     from ..utils.misc import check_deps
@@ -82,6 +83,16 @@ def build_workflow(config_file, retval):
 
     retval["workflow"] = init_fmriprep_wf()
 
+    # Check for FS license after building the workflow
+    if not check_valid_fs_license():
+        build_log.critical("""\
+ERROR: a valid license file is required for FreeSurfer to run. fMRIPrep looked for an existing \
+license file at several paths, in this order: 1) command line argument ``--fs-license-file``; \
+2) ``$FS_LICENSE`` environment variable; and 3) the ``$FREESURFER_HOME/license.txt`` path. Get it \
+(for free) by registering at https://surfer.nmr.mgh.harvard.edu/registration.html""")
+        retval["return_code"] = 126  # 126 == Command invoked cannot execute.
+        return retval
+
     # Check workflow for missing commands
     missing = check_deps(retval["workflow"])
     if missing:

fmriprep/config.py

@@ -122,8 +122,11 @@
     del _cgroup
 
 _fs_license = os.getenv('FS_LICENSE')
-if _fs_license is None and os.getenv('FREESURFER_HOME'):
-    _fs_license = os.path.join(os.getenv('FREESURFER_HOME'), 'license.txt')
+if not _fs_license and os.getenv('FREESURFER_HOME'):
+    _fs_home = os.getenv('FREESURFER_HOME')
+    if _fs_home and (Path(_fs_home) / "license.txt").is_file():
+        _fs_license = str(Path(_fs_home) / "license.txt")
+    del _fs_home
 
 _templateflow_home = Path(os.getenv(
     'TEMPLATEFLOW_HOME',
@@ -377,6 +380,9 @@ class execution(_Config):
     @classmethod
     def init(cls):
         """Create a new BIDS Layout accessible with :attr:`~execution.layout`."""
+        if cls.fs_license_file and Path(cls.fs_license_file).is_file():
+            os.environ["FS_LICENSE"] = str(cls.fs_license_file)
+
         if cls._layout is None:
             import re
             from bids.layout import BIDSLayout

wrapper/fmriprep_docker.py

@@ -225,6 +225,7 @@ def is_in_directory(filepath, directory):
 def get_parser():
     """Defines the command line interface of the wrapper"""
     import argparse
+    from functools import partial
 
     class ToDict(argparse.Action):
         def __call__(self, parser, namespace, values, option_string=None):
@@ -234,11 +235,21 @@ def __call__(self, parser, namespace, values, option_string=None):
                 d[k] = os.path.abspath(v)
             setattr(namespace, self.dest, d)
 
+    def _is_file(path, parser):
+        """Ensure a given path exists and it is a file."""
+        if not os.path.isfile(path):
+            raise parser.error(
+                "Path should point to a file (or symlink of file): <%s>." % path
+            )
+        return path
+
     parser = argparse.ArgumentParser(
         description=__doc__,
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
         add_help=False)
 
+    IsFile = partial(_is_file, parser=parser)
+
     # Standard FMRIPREP arguments
     parser.add_argument('bids_dir', nargs='?', type=os.path.abspath,
                         default='')
@@ -279,7 +290,7 @@ def __call__(self, parser, namespace, values, option_string=None):
                                  ', '.join(NONSTANDARD_REFERENCES)))
 
     g_wrap.add_argument(
-        '--fs-license-file', metavar='PATH', type=os.path.abspath,
+        '--fs-license-file', metavar='PATH', type=IsFile,
         default=os.getenv('FS_LICENSE', None),
         help='Path to FreeSurfer license key file. Get it (for free) by registering'
              ' at https://surfer.nmr.mgh.harvard.edu/registration.html')