Merge pull request #1830 from oesteban/fix/1777

FIX: Error during version check trying to access read-only file systems

Author: oesteban

.circleci/config.yml

@@ -226,6 +226,7 @@ jobs:
           no_output_timeout: 2h
           command: |
             docker run -ti --rm=false \
+              -e TEST_READONLY_FILESYSTEM=1 -v $HOME:/home/readonly:ro \
               --entrypoint="pytest" poldracklab/fmriprep:latest \
               /src/fmriprep/fmriprep -svx --doctest-modules
 

fmriprep/cli/tests/test_version.py

@@ -1,4 +1,5 @@
 """Test version checks."""
+from os import getenv
 from datetime import datetime
 from pathlib import Path
 from packaging.version import Version
@@ -161,3 +162,21 @@ def mock_get(*args, **kwargs):
         assert reason == test_reason
     else:
         assert reason is None
+
+
+def test_readonly(tmp_path, monkeypatch):
+    """Test behavior when $HOME/.cache/fmriprep/latest can't be written out."""
+    home_path = Path('/home/readonly') if getenv('TEST_READONLY_FILESYSTEM') \
+        else tmp_path
+    monkeypatch.setenv('HOME', str(home_path))
+    cachedir = home_path / '.cache'
+
+    if getenv('TEST_READONLY_FILESYSTEM') is None:
+        cachedir.mkdir(mode=0o555, exist_ok=True)
+
+    # Make sure creating the folder will raise the exception.
+    with pytest.raises(OSError):
+        (cachedir / 'fmriprep').mkdir(parents=True)
+
+    # Should not raise
+    check_latest()

fmriprep/cli/version.py

@@ -19,21 +19,25 @@ def check_latest():
     date = None
     outdated = None
     cachefile = Path.home() / '.cache' / 'fmriprep' / 'latest'
-    cachefile.parent.mkdir(parents=True, exist_ok=True)
-
     try:
-        latest, date = cachefile.read_text().split('|')
-    except Exception:
-        pass
-    else:
+        cachefile.parent.mkdir(parents=True, exist_ok=True)
+    except OSError:
+        cachefile = None
+
+    if cachefile and cachefile.exists():
         try:
-            latest = Version(latest)
-            date = datetime.strptime(date, DATE_FMT)
-        except (InvalidVersion, ValueError):
-            latest = None
+            latest, date = cachefile.read_text().split('|')
+        except Exception:
+            pass
         else:
-            if abs((datetime.now() - date).days) > RELEASE_EXPIRY_DAYS:
-                outdated = True
+            try:
+                latest = Version(latest)
+                date = datetime.strptime(date, DATE_FMT)
+            except (InvalidVersion, ValueError):
+                latest = None
+            else:
+                if abs((datetime.now() - date).days) > RELEASE_EXPIRY_DAYS:
+                    outdated = True
 
     if latest is None or outdated is True:
         try:
@@ -49,7 +53,7 @@ def check_latest():
         else:
             latest = None
 
-    if latest is not None:
+    if cachefile is not None and latest is not None:
         try:
             cachefile.write_text('|'.join(('%s' % latest, datetime.now().strftime(DATE_FMT))))
         except Exception: