Transfer of fasteners dependency to filelock (#4800)

- change fasteners to filelock for preventing race condition when generating offsets
- use mock in tests to emulate PermissionError when creating filelock

---------

Co-authored-by: Oliver Beckstein <[email protected]>
Co-authored-by: Yuxuan Zhuang <[email protected]>

Author: 3 people

.github/actions/setup-deps/action.yaml

@@ -21,8 +21,8 @@ inputs:
     default: 'codecov'
   cython:
     default: 'cython'
-  fasteners:
-    default: 'fasteners'
+  filelock:
+    default: 'filelock'
   griddataformats:
     default: 'griddataformats'
   gsd:
@@ -110,7 +110,7 @@ runs:
         CONDA_MIN_DEPS: |
           ${{ inputs.codecov }}
           ${{ inputs.cython }}
-          ${{ inputs.fasteners }}
+          ${{ inputs.filelock }}
           ${{ inputs.griddataformats }}
           ${{ inputs.hypothesis }}
           ${{ inputs.matplotlib }}

azure-pipelines.yml

@@ -90,7 +90,7 @@ jobs:
       scikit-learn
       tqdm
       threadpoolctl
-      fasteners
+      filelock
     displayName: 'Install dependencies'
   # for wheel install testing, we pin to an
   # older NumPy, the oldest version we support and that

maintainer/conda/environment.yml

@@ -7,7 +7,7 @@ dependencies:
   - codecov
   - cython
   - docutils
-  - fasteners
+  - filelock
   - griddataformats
   - gsd
   - h5py>=2.10

package/CHANGELOG

@@ -36,6 +36,7 @@ Enhancements
 
 
 Changes
+ * Changed `fasteners` dependency to  `filelock` (Issue #4797, PR #4800)
  * Codebase is now formatted with black (version `24`) (PR #4886)
 
 Deprecations

package/MDAnalysis/coordinates/XDR.py

@@ -38,7 +38,7 @@
 import numpy as np
 from os.path import getctime, getsize, isfile, split, join
 import warnings
-import fasteners
+from filelock import FileLock
 
 from . import base
 from ..lib.mdamath import triclinic_box
@@ -121,6 +121,8 @@ class XDRBaseReader(base.ReaderBase):
        Add a InterProcessLock when generating offsets
     .. versionchanged:: 2.4.0
        Use a direct read into ts attributes
+    .. versionchanged:: 2.9.0
+       Changed fasteners.InterProcessLock() to filelock.FileLock
     """
     @store_init_arguments
     def __init__(self, filename, convert_units=True, sub=None,
@@ -195,18 +197,18 @@ def _load_offsets(self):
 
         #  check if the location of the lock is writable.
         try:
-            with fasteners.InterProcessLock(lock_name) as filelock:
+            with FileLock(lock_name) as filelock:
                 pass
         except OSError as e:
             if isinstance(e, PermissionError) or e.errno == errno.EROFS:
                 warnings.warn(f"Cannot write lock/offset file in same location as "
                               f"{self.filename}. Using slow offset calculation.")
-                self._read_offsets(store=True)
+                self._read_offsets(store=False)
                 return
             else:
                 raise
 
-        with fasteners.InterProcessLock(lock_name) as filelock:
+        with FileLock(lock_name) as filelock:
             if not isfile(fname):
                 self._read_offsets(store=True)
                 return

package/pyproject.toml

@@ -38,7 +38,7 @@ dependencies = [
     'tqdm>=4.43.0',
     'threadpoolctl',
     'packaging',
-    'fasteners',
+    'filelock',
     'mda-xdrlib',
     'waterdynamics',
     'pathsimanalysis',

package/requirements.txt

@@ -1,7 +1,7 @@
 biopython>=1.80
 codecov
 cython
-fasteners
+filelock
 griddataformats
 gsd
 hypothesis

testsuite/MDAnalysisTests/coordinates/test_xdr.py

@@ -25,8 +25,10 @@
 
 import re
 import os
+import sys
 import shutil
 import subprocess
+import time
 from pathlib import Path
 
 import numpy as np
@@ -60,6 +62,7 @@
 from MDAnalysis.coordinates.base import Timestep
 from MDAnalysis.coordinates import XDR
 from MDAnalysisTests.util import get_userid
+from filelock import FileLock
 
 
 @pytest.mark.parametrize(
@@ -977,45 +980,39 @@ def test_unsupported_format(self, traj):
             reader = self._reader(traj)
         reader[idx_frame]
 
-    @pytest.mark.skipif(get_userid() == 0, reason="cannot readonly as root")
-    def test_persistent_offsets_readonly(self, tmpdir):
+    def test_persistent_offsets_readonly(self, tmpdir, trajectory):
         shutil.copy(self.filename, str(tmpdir))
 
-        if os.name == "nt":
-            # Windows platform has a unique way to deny write access
-            subprocess.call(
-                "icacls {fname} /deny Users:W".format(fname=tmpdir), shell=True
+        filename = str(tmpdir.join(os.path.basename(self.filename)))
+        print('filename', filename)
+        ref_offset = trajectory._xdr.offsets
+        # Mock filelock acquire to raise an error
+        with patch.object(FileLock, "acquire", side_effect=PermissionError):  # Simulate failure
+            with pytest.warns(UserWarning, match="Cannot write lock"):
+                reader = self._reader(filename)
+                saved_offsets = reader._xdr.offsets
+
+            # Check if offsets are handled properly and match reference offsets
+            assert_almost_equal(
+                saved_offsets,  # Compare with reference offsets
+                ref_offset,
+                err_msg="error loading frame offsets",
             )
-        else:
-            os.chmod(str(tmpdir), 0o555)
 
-        filename = str(tmpdir.join(os.path.basename(self.filename)))
-        # try to write a offsets file
-        with pytest.warns(
-            UserWarning, match="Couldn't save offsets"
-        ) and pytest.warns(UserWarning, match="Cannot write"):
-            self._reader(filename)
         assert_equal(os.path.exists(XDR.offsets_filename(filename)), False)
         # check the lock file is not created as well.
         assert_equal(
             os.path.exists(XDR.offsets_filename(filename, ending=".lock")),
             False,
         )
 
-        # pre-teardown permission fix - leaving permission blocked dir
-        # is problematic on py3.9 + Windows it seems. See issue
-        # [4123](https://github.com/MDAnalysis/mdanalysis/issues/4123)
-        # for more details.
-        if os.name == "nt":
-            subprocess.call(f"icacls {tmpdir} /grant Users:W", shell=True)
-        else:
-            os.chmod(str(tmpdir), 0o777)
-
-        shutil.rmtree(tmpdir)
-
-    def test_offset_lock_created(self):
+    @pytest.mark.skipif(
+        sys.platform.startswith("win"),
+        reason="The lock file only exists when it's locked in windows"
+    )
+    def test_offset_lock_created(self, traj):
         assert os.path.exists(
-            XDR.offsets_filename(self.filename, ending="lock")
+            XDR.offsets_filename(traj, ending="lock")
         )