ENH: Separate logic for identifying env vars into separate function. Save

original (un-expanded) default plugin destination in manifest

fsl/add_module/plugin_manifest.py

@@ -55,7 +55,7 @@ import urllib.request as urlrequest
 
 from typing import Union, Optional, List
 
-from fsl.add_module.routines import (expand, downloadFile)
+import fsl.add_module.routines as routines
 
 
 log = logging.getLogger(__name__)
@@ -96,7 +96,16 @@ class Plugin:
     destination : Union[None, str, pathlib.Path] = None
     """Installation/destination directory - where the plugin archive file
     will be extracted. May have a default value in the manifest, but may
-    be overridden at user request.
+    be overridden at user request. Any environment variables and the ``~``
+    character will be replaced with the variable value/user home directory.
+    """
+
+    origDestination : Union[None, str, pathlib.Path] = None
+    """Original value of ``destination``, before environment variable/tilde
+    expansion.
+
+    If the original destination contains any environment variables that
+    are not set, the user is warned.
     """
 
     archiveFile : Union[None, str, pathlib.Path] = None
@@ -142,7 +151,7 @@ class Manifest(dict):
 
         # support paths to local files
         if op.exists(url):
-            url         = expand(url)
+            url         = routines.expand(url)
             archiveFile = url
             url         = urlparse.urljoin(
                 'file:', urlrequest.pathname2url(url))
@@ -163,20 +172,23 @@ class Manifest(dict):
 
         # allow env vars and tilde in
         # JSON destination field
-        destination = expand(destination)
+        origDestination = destination
+        destination     = routines.expand(destination)
 
         if name in self:
             log.warning('Module with name %s [%s] already exists! Overwriting '
                         'with [%s]', name, self[name].url, url)
 
-        self[name] = Plugin(name,
-                            url,
-                            category,
-                            description,
-                            checksum,
-                            destination,
-                            archiveFile,
-                            None)
+        self[name] = Plugin(
+            name=name,
+            url=url,
+            category=category,
+            description=description,
+            checksum=checksum,
+            destination=destination,
+            origDestination=origDestination,
+            archiveFile=archiveFile,
+            checksumPassed=None)
 
         log.debug('Registered plugin" %s', self[name])
 
@@ -218,7 +230,7 @@ def downloadManifest(url : Union[str, pathlib.Path]) -> Manifest:
 
     with tempfile.TemporaryDirectory() as d:
         dest = op.join(d, 'manifest.txt')
-        downloadFile(url, dest, progress=False)
+        routines.downloadFile(url, dest, progress=False)
         rawmanifest = open(dest, 'rt').read()
 
     try:

fsl/add_module/routines.py

@@ -20,7 +20,7 @@ import                   pathlib
 import                   urllib
 import urllib.request as urlrequest
 
-from typing import Union, BinaryIO
+from typing import Union, List, BinaryIO
 
 try:                import progressbar
 except ImportError: progressbar = None
@@ -28,21 +28,39 @@ except ImportError: progressbar = None
 import fsl.add_module.admin as admin
 
 
-def expandvars(path: Union[str, pathlib.Path]) -> str:
-    """Alternative to ``os.path.expandvars`` which replaces unset variables
-    with an empty string.
+def envvars(path: Union[str, pathlib.Path]) -> List[str]:
+    """Identify all environment variables which are present in the given
+    file path/string. Variables of the form ``$VARNAME`` and ``${VARNAME}``
+    are identified.
+
+    :returns: A list of tuples, with each tuple containing:
+               - The full variable invocation, including the ``$`` and ``{}``
+                 characters (e.g. ``'${VARNAME}'``)
+               - The variable name (e.g. ``'VARNAME'``)
     """
 
     path     = str(path)
+    matches  = []
     patterns = [
         r'(\$\{(\w+)\})',
         r'(\$(\w+))',
     ]
 
     for pat in patterns:
-        matches = re.findall(pat, path)
-        for fullmatch, name in matches:
-            path = path.replace(fullmatch, os.environ.get(name, ''))
+        matches.extend(re.findall(pat, path))
+
+    return matches
+
+
+def expandvars(path: Union[str, pathlib.Path]) -> str:
+    """Alternative to ``os.path.expandvars`` which replaces unset variables
+    with an empty string.
+    """
+
+    path = str(path)
+
+    for fullmatch, name in envvars(path):
+        path = path.replace(fullmatch, os.environ.get(name, ''))
 
     return path