Consolidate logging messages into shell normalization processes

- prevent case where normalization could occur disregarding the `disable_normalization` setting - consolidate gitbash path normalization code Signed-off-by: javrin <jawabiscuit@users.noreply.github.com>
AcademySoftwareFoundation · Sep 15, 2023 · a9aeb94 · a9aeb94
1 parent f5e9b7a
commit a9aeb94
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 74 deletions.
diff --git a/src/rezplugins/shell/_utils/powershell_base.py b/src/rezplugins/shell/_utils/powershell_base.py
@@ -250,15 +250,12 @@ def normalize_path(self, path):
         if config.disable_normalization:
             return path
 
+        # TODO: Is this necessary?
         if platform_.name == "windows":
             converted_path = convert_path(path, 'windows')
             if path != converted_path:
-                print_debug(
-                    'Path converted: {} -> {}'.format(path, converted_path)
-                )
-                self._addline(
-                    '# Path converted: {} -> {}'.format(path, converted_path)
-                )
+                print_debug("Path normalized: {} -> {}".format(path, converted_path))
+                self._addline("# Path normalized: {} -> {}".format(path, converted_path))
             return converted_path
 
         else:
@@ -274,14 +271,6 @@ def setenv(self, key, value):
         is_path = self._is_pathed_key(key)
         new_value = self.escape_string(value, is_path=is_path)
 
-        if is_path and value != new_value:
-            print_debug(
-                'Path changed: {} -> {}'.format(value, new_value)
-            )
-            self._addline(
-                '# Path value changed: {} -> {}'.format(value, new_value)
-            )
-
         self._addline(
             'Set-Item -Path "Env:{0}" -Value "{1}"'.format(key, new_value)
         )
@@ -290,11 +279,6 @@ def prependenv(self, key, value):
         is_path = self._is_pathed_key(key)
         new_value = self.escape_string(value, is_path=is_path)
 
-        if is_path and value != new_value:
-            self._addline(
-                '# Path value changed: {} -> {}'.format(value, new_value)
-            )
-
         # Be careful about ambiguous case in pwsh on Linux where pathsep is :
         # so that the ${ENV:VAR} form has to be used to not collide.
         self._addline(
@@ -304,20 +288,16 @@ def prependenv(self, key, value):
 
     def appendenv(self, key, value):
         is_path = self._is_pathed_key(key)
-        new_value = self.escape_string(value, is_path=is_path)
-
-        if is_path and value != new_value:
-            self._addline(
-                '# Path value changed: {} -> {}'.format(value, new_value)
-            )
+        # Doesn't just escape, but can also perform path normalization
+        modified_value = self.escape_string(value, is_path=is_path)
 
         # Be careful about ambiguous case in pwsh on Linux where pathsep is :
         # so that the ${ENV:VAR} form has to be used to not collide.
         # The nested Get-ChildItem call is set to SilentlyContinue to prevent
         # an exception of the Environment Variable is not set already
         self._addline(
             'Set-Item -Path "Env:{0}" -Value ((Get-ChildItem -ErrorAction SilentlyContinue "Env:{0}").Value + "{1}{2}")'
-            .format(key, os.path.pathsep, value))
+            .format(key, os.path.pathsep, modified_value))
 
     def unsetenv(self, key):
         self._addline(

diff --git a/src/rezplugins/shell/_utils/windows.py b/src/rezplugins/shell/_utils/windows.py
@@ -51,9 +51,6 @@ def _repl(m):
         # Backslash -> fwdslash
         new_path = new_path.replace('\\', '/')
 
-    if path != new_path:
-        print_debug('Path converted: {} -> {}'.format(path, new_path))
-
     return new_path
 
 

diff --git a/src/rezplugins/shell/cmd.py b/src/rezplugins/shell/cmd.py
@@ -302,11 +302,7 @@ def normalize_path(self, path):
         converted_path = convert_path(path, 'windows')
 
         if path != converted_path:
-            self._addline(
-                'REM Path converted: {} -> {}'.format(
-                    path, converted_path
-                )
-            )
+            self._addline("REM normalized path: {!r} -> {}".format(path, converted_path))
 
         return converted_path
 

diff --git a/src/rezplugins/shell/csh.py b/src/rezplugins/shell/csh.py
@@ -160,17 +160,7 @@ def _saferefenv(self, key):
         self._addline("if (!($?%s)) setenv %s" % (key, key))
 
     def setenv(self, key, value):
-        is_path = self._is_pathed_key(key) or self._is_shell_pathed_key(key)
         new_value = self.escape_string(value, is_path=self._is_pathed_key(key))
-
-        if is_path and value != new_value:
-            print_debug(
-                'Path changed: {} -> {}'.format(value, new_value)
-            )
-            self._addline(
-                '# Path value changed: {} -> {}'.format(value, new_value)
-            )
-
         self._addline('setenv %s %s' % (key, new_value))
 
     def unsetenv(self, key):

diff --git a/src/rezplugins/shell/gitbash.py b/src/rezplugins/shell/gitbash.py
@@ -7,11 +7,12 @@
 import re
 import os.path
 import subprocess
+
 from rez.config import config
 from rezplugins.shell.bash import Bash
 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_debug, print_warning
 from rez.util import dedup
 
 if platform_.name == 'windows':
@@ -112,18 +113,10 @@ def as_shell_path(self, path):
         Returns:
             (str): Transformed file path.
         """
-        # Prevent path conversion if normalization is disabled in the config.
-        if config.disable_normalization:
-            return path
-
-        converted_path = convert_path(path, mode='mixed', force_fwdslash=True)
-        if path != converted_path:
-            self._addline(
-                '# Path converted: {} -> {}'.format(path, converted_path)
-            )
+        converted_path = self.normalize_path(path, mode="mixed")
         return converted_path
 
-    def normalize_path(self, path):
+    def normalize_path(self, path, mode="unix"):
         """Normalize the path to fit the environment.
         For example, POSIX paths, Windows path, etc. If no transformation is
         necessary, just return the path.
@@ -138,14 +131,18 @@ def normalize_path(self, path):
         if config.disable_normalization:
             return path
 
-        converted_path = convert_path(path, mode='unix', force_fwdslash=True)
-        if path != converted_path:
+        normalized_path = convert_path(path, mode=mode, force_fwdslash=True)
+        if path != normalized_path:
+            print_debug(
+                "path normalized: {!r} -> {}".format(path, normalized_path)
+            )
             self._addline(
-                '# Path converted: {} -> {}'.format(path, converted_path)
+                "# path normalized: {!r} -> {}".format(path, normalized_path)
             )
-        return converted_path
 
-    def normalize_paths(self, value):
+        return normalized_path
+
+    def normalize_paths(self, path):
         """
         This is a bit tricky in the case of gitbash. The problem we hit is that
         our pathsep is ':', _but_ pre-normalised paths also contain ':' (eg
@@ -156,13 +153,25 @@ def normalize_paths(self, value):
         normalize_path() still does drive-colon replace also - it needs to
         behave correctly if passed a string like C:\foo.
         """
+        if config.disable_normalization:
+            return path
+
         def lowrepl(match):
             if match:
-                return '/{}/'.format(match.group(1).lower())
+                return "/{}/".format(match.group(1).lower())
 
         # C:\ ==> /c/
-        value2 = self._drive_regex.sub(lowrepl, value).replace('\\', '/')
-        return value2
+        normalized_path = self._drive_regex.sub(lowrepl, path).replace("\\", "/")
+
+        if path != normalized_path:
+            print_debug(
+                "path normalized: {!r} -> {}".format(path, normalized_path)
+            )
+            self._addline(
+                "# path normalized: {!r} -> {}".format(path, normalized_path)
+            )
+
+        return normalized_path
 
     def shebang(self):
         self._addline('#! /usr/bin/env bash')

diff --git a/src/rezplugins/shell/sh.py b/src/rezplugins/shell/sh.py
@@ -106,24 +106,16 @@ def _bind_interactive_rez(self):
 
     def setenv(self, key, value):
         is_implicit = key == 'REZ_USED_IMPLICIT_PACKAGES'
-        is_path = self._is_pathed_key(key) or self._is_shell_pathed_key(key)
 
-        new_value = self.escape_string(
+        # Doesn't just escape, but can also perform path normalization
+        modified_value = self.escape_string(
             value,
             is_path=self._is_pathed_key(key),
             is_shell_path=self._is_shell_pathed_key(key),
             is_implicit=is_implicit,
         )
 
-        if is_path and value != new_value:
-            print_debug(
-                'Path value changed: {} -> {}'.format(value, new_value)
-            )
-            self._addline(
-                '# Path value changed: {} -> {}'.format(value, new_value)
-            )
-
-        self._addline('export %s=%s' % (key, new_value))
+        self._addline("export %s=%s" % (key, modified_value))
 
     def unsetenv(self, key):
         self._addline("unset %s" % key)
@@ -155,6 +147,7 @@ def escape_string(
                 if is_shell_path:
                     txt = self.as_shell_path(txt)
                 elif is_path:
+                    # potentially calls plugin shell's normalize
                     txt = self.normalize_paths(txt)
 
                 txt = txt.replace('\\', '\\\\')