From 3a874cf2f4048c017cbb5a438474ad382d64eee2 Mon Sep 17 00:00:00 2001
From: Martin Craig <martin.craig@eng.ox.ac.uk>
Date: Tue, 21 Apr 2020 12:30:30 +0100
Subject: [PATCH] Implement revised version of WSL interface using \\wsl$\
 mapping This requires  WSL 2.0 but enables FSLDIR to be a genuine WIndows
 path so that atlases and standard data can be loaded transparently. It also
 means we no longer need FSLWSL environment variable, just to be a bit careful
 with path mapping

---
 fsl/utils/platform.py  | 56 ++++++++++++++++++++++++++++++++++++++++--
 fsl/utils/run.py       | 50 +++++++++++++++++--------------------
 tests/test_platform.py | 31 +++++++++++++++++++----
 tests/test_run.py      |  4 ---
 4 files changed, 102 insertions(+), 39 deletions(-)

diff --git a/fsl/utils/platform.py b/fsl/utils/platform.py
index 34f60162c..8599928e1 100644
--- a/fsl/utils/platform.py
+++ b/fsl/utils/platform.py
@@ -17,6 +17,7 @@ import os
 import os.path as op
 import sys
 import importlib
+import re
 
 import fsl.utils.notifier as notifier
 
@@ -285,7 +286,6 @@ class Platform(notifier.Notifier):
         """
         return os.environ.get('FSLDIR', None)
 
-
     @property
     def fsldevdir(self):
         """The FSL development directory location. """
@@ -294,7 +294,7 @@ class Platform(notifier.Notifier):
     @property
     def fslwsl(self):
         """Boolean flag indicating whether FSL is installed in Windows Subsystem for Linux """
-        return os.environ.get('FSLWSL', "0") == "1"
+        return self.fsldir is not None and self.fsldir.startswith("\\\\wsl$\\")
 
     @fsldir.setter
     def fsldir(self, value):
@@ -405,6 +405,58 @@ class Platform(notifier.Notifier):
         """
         return self.__glIsSoftware
 
+    def wslpath(self, winpath):
+        """ 
+        Convert Windows path (or a command line argument containing a Windows path) 
+        to the equivalent WSL path (e.g. ``c:\\Users`` -> ``/mnt/c/Users``). Also supports
+        paths in the form ``\\wsl$\\(distro)\\users\\...``
+        
+        :param winpath: Command line argument which may (or may not) contain a Windows path. It is assumed to be 
+                        either of the form <windows path> or --<arg>=<windows path>. Note that we don't need to 
+                        handle --arg <windows path> or -a <windows path> since in these cases the argument 
+                        and the path will be parsed as separate entities. 
+        :return: If ``winpath`` matches a Windows path, the converted argument (including the --<arg>= portion). 
+                 Otherwise returns ``winpath`` unchanged.
+        """
+        match = re.match(r"^(--[\w-]+=)?\\\\wsl\$\\[^\\]+(\\.*)$", winpath)
+        if match:
+            arg, path = match.group(1, 2)
+            if arg is None:
+                arg = ""
+            return arg + path.replace("\\", "/")
+
+        match = re.match(r"^(--[\w-]+=)?([a-zA-z]):(.+)$", winpath)
+        if match:
+            arg, drive, path = match.group(1, 2, 3)
+            if arg is None:
+                arg = ""
+            return arg + "/mnt/" + drive.lower() + path.replace("\\", "/") 
+
+        return winpath
+
+    def winpath(self, wslpath):
+        """
+        Convert a WSL-local filepath (for example ``/usr/local/fsl/``) into a path that can be used from
+        Windows.
+
+        If ``self.fslwsl`` is ``False``, simply returns ``wslpath`` unmodified
+        Otherwise, uses ``FSLDIR`` to deduce the WSL distro in use for FSL.
+        This requires WSL2 which supports the ``\\wsl$\`` network path.
+        wslpath is assumed to be an absolute path.
+        """
+        if not self.fslwsl:
+            return wslpath
+        else:
+            match = re.match(r"^\\\\wsl\$\\([^\\]+).*$", self.fsldir)
+            if match:
+                distro = match.group(1)
+            else:
+                distro = None
+
+            if distro is None:
+                raise RuntimeError("No WSL installations found at \\wsl$\ - a valid WSL 2.0 installation is required")
+            else:
+                return "\\\\wsl$\\" + distro + wslpath.replace("/", "\\")
 
 platform = Platform()
 """An instance of the :class:`Platform` class. Feel free to create your own
diff --git a/fsl/utils/run.py b/fsl/utils/run.py
index abba42f0e..2523f50dc 100644
--- a/fsl/utils/run.py
+++ b/fsl/utils/run.py
@@ -29,7 +29,6 @@ import               collections
 import subprocess as sp
 import os.path    as op
 import               os
-import               re
 
 import               six
 
@@ -392,44 +391,39 @@ def wslcmd(cmdpath, *args):
              which when executed will run the command in WSL. Windows paths in the argument list will
              be converted to WSL paths. If ``cmdpath`` was not executable in WSL, returns None
     """
-    # Check if command exists in WSL (translating Windows path separators to Unix as os.path.join 
-    # may have inserted some backslashes)
-    cmdpath = cmdpath.replace("\\", "/")
+    # Check if command exists in WSL (remembering that the command path may include FSLDIR which
+    # is a Windows path)
+    cmdpath = fslplatform.wslpath(cmdpath)
     retcode = sp.call(["wsl", "test", "-x", cmdpath])
     if retcode == 0:
         # Form a new argument list and convert any Windows paths in it into WSL paths
-        wslargs = [wslpath(arg) for arg in args]
+        wslargs = [fslplatform.wslpath(arg) for arg in args]
         wslargs[0] = cmdpath
+        local_fsldir = fslplatform.wslpath(fslplatform.fsldir)
+        if fslplatform.fsldevdir:
+            local_fsldevdir = fslplatform.wslpath(fslplatform.fsldevdir)
+        else:
+            local_fsldevdir = None
         # Prepend important environment variables - note that it seems we cannot
-        # use WSLENV for this due to its insistance on path mapping.
-        return [
+        # use WSLENV for this due to its insistance on path mapping. FIXME FSLDEVDIR?
+        local_path = "$PATH"
+        if local_fsldevdir:
+            local_path += ":%s/bin" % local_fsldevdir
+        local_path += ":%s/bin" % local_fsldir
+        prepargs = [
             "wsl",
-            "PATH=$PATH:%s/bin" % fslplatform.fsldir,
-            "FSLDIR=%s" % fslplatform.fsldir,
+            "PATH=%s" % local_path,
+            "FSLDIR=%s" % local_fsldir,
             "FSLOUTPUTTYPE=%s" % os.environ.get("FSLOUTPUTTYPE", "NIFTI_GZ")
-        ] + wslargs
+        ]
+        if local_fsldevdir:
+            prepargs.append("FSLDEVDIR=%s" % local_fsldevdir)
+
+        return prepargs + wslargs
     else:
         # Command was not found in WSL with this path
         return None
 
-def wslpath(patharg):
-    """ 
-    Convert a command line argument containing a Windows path to the equivalent WSL path (e.g. ``c:\\Users`` -> ``/mnt/c/Users``) 
-    
-    :param patharg: Command line argument which may (or may not) contain a Windows path. It is assumed to be 
-                    either of the form <windows path> or --<arg>=<windows path>
-    :return: If ``patharg`` matches a Windows path, the converted argument (including the --<arg>= portion). Otherwise
-             returns ``patharg`` unchanged.
-    """
-    match = re.match("^(--[\w-]+=)?([a-zA-z]):(.+)$", patharg)
-    if match:
-        arg, drive, path = match.group(1, 2, 3)
-        if arg is None:
-            arg = ""
-        return arg + "/mnt/" + drive.lower() + path.replace("\\", "/") 
-    else:
-        return patharg
-
 def wait(job_ids):
     """Proxy for :func:`.fslsub.wait`. """
     return fslsub.wait(job_ids)
diff --git a/tests/test_platform.py b/tests/test_platform.py
index 9fbb004cb..46dad0ce5 100644
--- a/tests/test_platform.py
+++ b/tests/test_platform.py
@@ -214,11 +214,32 @@ def test_detect_ssh():
         assert not p.inVNCSession
 
 def test_fslwsl():
-
-     with mock.patch.dict('os.environ', **{ 'FSLWSL' : '1'}):
-        p = fslplatform.Platform()
+    """
+    Note that ``Platform.fsldir`` requires the directory in ``FSLDIR`` to exist and
+    sets ``FSLDIR`` to ``None`` if it doesn't. So we create a ``Platform`` first 
+    and then overwrite ``FSLDIR``. This is a bit of a hack but the logic we are testing
+    here is whether ``Platform.fslwsl`` recognizes a WSL ``FSLDIR`` string
+    """
+    p = fslplatform.Platform()
+    with mock.patch.dict('os.environ', **{ 'FSLDIR' : '\\\\wsl$\\my cool linux distro v1.0\\usr\\local\\fsl'}):
         assert p.fslwsl
 
-     with mock.patch.dict('os.environ', **{ 'FSLWSL' : '0'}):
-        p = fslplatform.Platform()
+    with mock.patch.dict('os.environ', **{ 'FSLDIR' : '/usr/local/fsl'}):
         assert not p.fslwsl
+
+def test_wslpath():
+    p = fslplatform.Platform()
+    assert p.wslpath('c:\\Users\\Fishcake\\image.nii.gz') == '/mnt/c/Users/Fishcake/image.nii.gz'
+    assert p.wslpath('--input=x:\\transfers\\scratch\\image_2.nii') == '--input=/mnt/x/transfers/scratch/image_2.nii'
+    assert p.wslpath('\\\\wsl$\\centos 7\\users\\fsl\\file.nii') == '/users/fsl/file.nii'
+    assert p.wslpath('--file=\\\\wsl$\\centos 7\\home\\fsl\\img.nii.gz') == '--file=/home/fsl/img.nii.gz'
+
+def test_winpath():
+    """
+    See comment for ``test_fslwsl``
+    """
+    p = fslplatform.Platform()
+    with mock.patch.dict('os.environ', **{ 'FSLDIR' : '\\\\wsl$\\my cool linux distro v2.0\\usr\\local\\fsl'}):
+        assert p.winpath("/home/fsl/myfile.dat") == '\\\\wsl$\\my cool linux distro v2.0\\home\\fsl\\myfile.dat'
+    with mock.patch.dict('os.environ', **{ 'FSLDIR' : '/opt/fsl'}):
+        assert p.winpath("/home/fsl/myfile.dat") == '/home/fsl/myfile.dat'
diff --git a/tests/test_run.py b/tests/test_run.py
index 39df3603c..363d4489e 100644
--- a/tests/test_run.py
+++ b/tests/test_run.py
@@ -410,7 +410,3 @@ def test_run_logcmd():
 
         assert stdout                         == expstdout
         assert open('my_stdout', 'rt').read() == expcmd + expstdout
-
-def test_wslpath():
-    assert run.wslpath('c:\\Users\\Fishcake\\image.nii.gz') == '/mnt/c/Users/Fishcake/image.nii.gz'
-    assert run.wslpath('--input=x:\\transfers\\scratch\\image_2.nii') == '--input=/mnt/x/transfers/scratch/image_2.nii'
-- 
GitLab