Implement shell env var sep settings

Signed-off-by: javrin <jawabiscuit@users.noreply.github.com>

src/rez/shells.py

@@ -76,6 +76,9 @@ class Shell(ActionInterpreter):
     schema_dict = {
         "prompt": basestring}
 
+    env_sep_map_setting = "env_var_separators"
+    shell_env_sep_map_setting = "shell_env_var_separators"
+
     @classmethod
     def name(cls):
         """Plugin name.
@@ -136,6 +139,56 @@ def get_syspaths(cls):
     def __init__(self):
         self._lines = []
         self.settings = config.plugins.shell[self.name()]
+        self.env_sep_map = self._get_env_sep_map()
+        self.validate_env_sep_map()
+
+    def _global_env_seps(self):
+        setting = self.env_sep_map_setting
+        value = config.get(setting, {})
+        return value
+
+    def _shell_env_seps(self):
+        shell = self.name()
+        setting = self.shell_env_sep_map_setting
+        values = config.get(setting, {})
+        value = values.get(shell, {})
+        return value
+
+    def _get_env_sep_map(self):
+        """
+        Get a dict of environment variable names to path separators.
+        """
+        if getattr(self, "_env_sep_map", None):
+            return self.env_sep_map
+
+        env_seps = {}
+        global_env_seps = self._global_env_seps()
+        shell_env_seps = self._shell_env_seps()
+
+        # Check for conflicting values and log debug info
+        if global_env_seps and shell_env_seps:
+            for var, pathsep in global_env_seps.items():
+                shell_pathsep = shell_env_seps.get(var, "")
+                if shell_pathsep and shell_pathsep != pathsep:
+                    log(
+                        "'%s' is configured in '%s' and configured in "
+                        "'%s'. In this case, the shell setting '%s' will "
+                        "trump the global setting '%s'",
+                        var,
+                        self.env_sep_map_setting,
+                        self.shell_env_sep_map_setting,
+                        shell_pathsep,
+                        pathsep,
+                    )
+
+        # Apply shell settings, overriding global settings
+        for var, pathsep in shell_env_seps.items():
+            env_seps[var] = pathsep
+
+        return env_seps
+
+    def validate_env_sep_map(self):
+        pass
 
     def _addline(self, line):
         self._lines.append(line)

src/rez/tests/test_utils.py

@@ -56,39 +56,39 @@ class TestPathConversion(TestBase):
     def test_convert_windows(self):
         """Test the path conversion to windows style."""
         test_path = r'C:\foo/bar/spam'
-        converted_path = cygpath.convert_path(test_path, 'windows')
+        converted_path = cygpath.convert(test_path, mode='windows')
         expected_path = r'C:\foo\bar\spam'
 
         self.assertEqual(converted_path, expected_path)
 
     def test_convert_unix(self):
         """Test the path conversion to unix style."""
         test_path = r'C:\foo\bar\spam'
-        converted_path = cygpath.convert_path(test_path, 'unix')
+        converted_path = cygpath.convert(test_path)
         expected_path = r'/c/foo/bar/spam'
 
         self.assertEqual(converted_path, expected_path)
 
     def test_convert_mixed(self):
         """Test the path conversion to mixed style."""
         test_path = r'C:\foo\bar\spam'
-        converted_path = cygpath.convert_path(test_path, 'unix')
+        converted_path = cygpath.convert(test_path)
         expected_path = r'/c/foo/bar/spam'
 
         self.assertEqual(converted_path, expected_path)
 
     def test_convert_unix_forced_fwdslash(self):
         """Test the path conversion to unix style."""
         test_path = r'C:\foo\bar\spam'
-        converted_path = cygpath.convert_path(test_path, 'unix', force_fwdslash=True)
+        converted_path = cygpath.convert(test_path, force_fwdslash=True)
         expected_path = r'/c/foo/bar/spam'
 
         self.assertEqual(converted_path, expected_path)
 
     def test_convert_mixed_forced_fwdslash(self):
         """Test the path conversion to mixed style while forcing fwd slashes."""
         test_path = r'C:\foo\bar\spam'
-        converted_path = cygpath.convert_path(test_path, 'mixed', force_fwdslash=True)
+        converted_path = cygpath.convert(test_path, mode='mixed', force_fwdslash=True)
         expected_path = r'C:/foo/bar/spam'
 
         self.assertEqual(converted_path, expected_path)

src/rez/utils/cygpath.py

@@ -10,12 +10,14 @@
 import os
 import re
 
+from rez.utils.logging_ import print_debug
+
 _drive_start_regex = re.compile(r"^([A-Za-z]):\\")
 _drive_regex_mixed = re.compile(r"([a-z]):/")
 _env_var_regex = re.compile(r"%([^%]*)%")
 
 
-def convert_path(path, mode='unix', force_fwdslash=False):
+def convert_path(path, mode='unix', env_var_seps=None, force_fwdslash=False):
     r"""Convert a path to unix style or windows style as per cygpath rules.
 
     Args:
@@ -38,11 +40,14 @@ def _repl(m):
 
     path = _env_var_regex.sub(_repl, path)
 
-    # Ensure the correct env var separator is being used in the case of
-    # `PYTHONPATH` in gitbash.
-    env_var_regex = r"(\$\{PYTHONPATH\})(:)"
-    env_sep_subst = "\\1;"
-    path = re.sub(env_var_regex, env_sep_subst, path, 0)
+    env_var_seps = env_var_seps or {}
+    for var, sep in env_var_seps.items():
+        start = path
+        regex = r"(\$\{%s\})([:;])" % var
+        path = re.sub(regex, "\\1%s" % sep, path, 0)
+        if path != start:
+            print_debug("cygpath convert_path() path in: {!r}".format(start))
+            print_debug("cygpath convert_path() path out: {!r}".format(path))
 
     # Convert the path based on mode.
     if mode == 'mixed':

src/rezplugins/shell/gitbash.py

@@ -14,7 +14,7 @@
 from rez.utils.cygpath import convert_path
 from rez.utils.execution import Popen
 from rez.utils.platform_ import platform_
-from rez.utils.logging_ import print_warning
+from rez.utils.logging_ import print_error, print_warning
 from rez.util import dedup
 from ._utils.windows import get_syspaths_from_registry
 
@@ -105,6 +105,82 @@ def get_syspaths(cls):
         cls.syspaths = paths
         return cls.syspaths
 
+    def validate_env_sep_map(self):
+        env_var_seps = self.env_sep_map
+        shell = self.name()
+        shell_setting = self.shell_env_sep_map_setting
+        py_path_var, py_path_sep = ("PYTHONPATH", ";")
+
+        # Begin validation, check special case for PYTHONPATH in gitbash
+        if env_var_seps:
+            if py_path_var not in env_var_seps:
+                print_error(
+                    "'%s' is not configured in '%s'. This is required for "
+                    "python to work correctly. Please add '%s' to %s['%s'] and "
+                    "set it to '%s' for the best experience working with  %s ",
+                    py_path_var,
+                    shell_setting,
+                    py_path_var,
+                    shell_setting,
+                    shell,
+                    py_path_sep,
+                    shell,
+                )
+        else:
+            print_error(
+                "'%s' is improperly configured! '%s' must be configured and "
+                "contain '%s' and set to '%s' for python to function and rez "
+                "in %s to work as expected.",
+                shell,
+                shell_setting,
+                py_path_var,
+                py_path_sep,
+                shell,
+            )
+
+        env_seps = self._global_env_seps()
+        shell_env_seps = self._shell_env_seps()
+        setting = self.env_sep_map_setting
+
+        is_configured = (env_seps and py_path_var in env_seps)
+        shell_is_configured = (shell_env_seps and py_path_var in shell_env_seps)
+
+        # If `shell_path_vars` is not configured for `PYTHONPATH`
+        # ensure `env_var_separators` is configured with `PYTHONPATH` set to `;`
+        # Otherwise communicate to the user that there's a configuration error.
+        if is_configured and not shell_is_configured:
+            if env_seps[py_path_var] != py_path_sep:
+                print_error(
+                    "'%s' is configured in '%s' but is not configured in '%s'. "
+                    "This is required for python to work correctly. Please add "
+                    "'%s' to %s['%s'] and set it to '%s' for the best "
+                    "experience working with  %s",
+                    py_path_var,
+                    setting,
+                    shell_setting,
+                    py_path_var,
+                    shell_setting,
+                    shell,
+                    py_path_sep,
+                    shell,
+                )
+            else:
+                print_warning(
+                    "'%s' is configured in '%s' but is not configured in '%s'. "
+                    "Using rez with gitbash will probably still work but "
+                    "configuration is technically incorrect and may cause "
+                    "problems. Please add '%s' to %s['%s'] "
+                    "and set it to '%s' for %s to ensure the best experience.",
+                    py_path_var,
+                    setting,
+                    shell_setting,
+                    py_path_var,
+                    shell_setting,
+                    shell,
+                    py_path_sep,
+                    shell,
+                )
+
     def as_path(self, path):
         """Return the given path as a system path.
         Used if the path needs to be reformatted to suit a specific case.
@@ -126,10 +202,23 @@ def as_shell_path(self, path):
         Returns:
             (str): Transformed file path.
         """
-        converted_path = self.normalize_path(path, mode="mixed")
-        return converted_path
+        # Prevent path conversion if normalization is disabled in the config.
+        if not config.enable_path_normalization:
+            return path
+
+        normalized_path = convert_path(
+            path, env_var_seps=self.env_sep_map, mode="mixed", force_fwdslash=True
+        )
+
+        if path != normalized_path:
+            log("GitBash as_shell_path()")
+            log("path normalized: {!r} -> {!r}".format(path, normalized_path))
+            self._addline(
+                "# path normalized: {!r} -> {!r}".format(path, normalized_path)
+            )
+        return normalized_path
 
-    def normalize_path(self, path, mode="unix"):
+    def normalize_path(self, path):
         """Normalize the path to fit the environment.
         For example, POSIX paths, Windows path, etc. If no transformation is
         necessary, just return the path.
@@ -144,7 +233,7 @@ def normalize_path(self, path, mode="unix"):
         if not config.enable_path_normalization:
             return path
 
-        normalized_path = convert_path(path, mode=mode, force_fwdslash=True)
+        normalized_path = convert_path(path, mode="unix", force_fwdslash=True)
         if path != normalized_path:
             log("GitBash normalize_path()")
             log("path normalized: {!r} -> {!r}".format(path, normalized_path))