From 500910708f601e99744f665acb3815d5969da9f7 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Sat, 2 Apr 2022 11:52:27 +1100 Subject: [PATCH 1/8] -added new pathed_env_vars config setting -added shell normalize_path method (does nothing as yet) --- src/rez/config.py | 1 + src/rez/rex.py | 33 +++++++++++++++++++ src/rez/rezconfig.py | 9 +++++ .../shell/_utils/powershell_base.py | 14 ++++++++ src/rezplugins/shell/cmd.py | 1 + src/rezplugins/shell/csh.py | 1 + src/rezplugins/shell/sh.py | 1 + 7 files changed, 60 insertions(+) diff --git a/src/rez/config.py b/src/rez/config.py index a2ca1415e..adfdfe1c2 100644 --- a/src/rez/config.py +++ b/src/rez/config.py @@ -344,6 +344,7 @@ def _parse_env_var(self, value): "resetting_variables": StrList, "release_hooks": StrList, "context_tracking_context_fields": StrList, + "pathed_env_vars": StrList, "prompt_release_message": Bool, "critical_styles": OptionalStrList, "error_styles": OptionalStrList, diff --git a/src/rez/rex.py b/src/rez/rex.py index 26f6a0ff7..8d549762d 100644 --- a/src/rez/rex.py +++ b/src/rez/rex.py @@ -8,6 +8,7 @@ import sys import re import traceback +from fnmatch import fnmatch from contextlib import contextmanager from string import Formatter @@ -550,9 +551,41 @@ def escape_string(self, value): Args: value (str or `EscapedString`): String to escape. + + Returns: + str: The escaped string. """ return str(value) + def normalize_path(self, path): + """Normalize a path. + + Change `path` to a valid filepath representation for this interpreter. + + Note: + Assume that `path` has already been escaped at this point. This will + matter for edge cases, like a path on linux containing an escaped + back slash. + + Args: + path (str): A filepath which may be in posix format, or windows + format, or some combination of the two. For eg, a string like + `{root}/bin` on windows will evaluate to `C:\\.../bin` - in this + case, the `cmd` shell would want to normalize this and convert + to all forward slashes. + + Returns: + str: The normalized path. + """ + return path + + def normalize_if_path(self, key, value): + """Normalize value if it's a path. + """ + if not any(fnmatch(key, x) for x in config.pathed_env_vars): + return value + return self.normalize_path(value) + # --- internal commands, not exposed to public rex API def _saferefenv(self, key): diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index 3ca79416e..d16625c98 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -524,6 +524,15 @@ "DOXYGEN_TAGFILES": " ", } +# This setting identifies path-like environment variables. This is required +# because some shells need to apply path normalization. For example, the command +# `env.PATH.append("{root}/bin")` will be normalized to (eg) `C:\...\bin` in a +# `cmd` shell on Windows. Note that wildcards are supported. If this setting is +# not correctly configured, then your shell may not be guaranteed to work correctly. +pathed_env_vars = [ + "*PATH" +] + # Defines what suites on $PATH stay visible when a new rez environment is resolved. # Possible values are: # - "never": Don"t attempt to keep any suites visible in a new env diff --git a/src/rezplugins/shell/_utils/powershell_base.py b/src/rezplugins/shell/_utils/powershell_base.py index 475e5f7c5..9cc8bdaca 100644 --- a/src/rezplugins/shell/_utils/powershell_base.py +++ b/src/rezplugins/shell/_utils/powershell_base.py @@ -246,10 +246,24 @@ def shebang(self): def setenv(self, key, value): value = self.escape_string(value) + value = self.normalize_if_path(key, value) self._addline('Set-Item -Path "Env:{0}" -Value "{1}"'.format(key, value)) + def prependenv(self, key, value): + value = self.escape_string(value) + value = self.normalize_if_path(key, 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( + 'Set-Item -Path "Env:{0}" -Value ("{1}{2}" + (Get-ChildItem "Env:{0}").Value)'.format( + key, value, os.path.pathsep) + ) + def appendenv(self, key, value): value = self.escape_string(value) + value = self.normalize_if_path(key, 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( diff --git a/src/rezplugins/shell/cmd.py b/src/rezplugins/shell/cmd.py index 13dbb0665..6ad083c78 100644 --- a/src/rezplugins/shell/cmd.py +++ b/src/rezplugins/shell/cmd.py @@ -305,6 +305,7 @@ def shebang(self): def setenv(self, key, value): value = self.escape_string(value) + value = self.normalize_if_path(key, value) self._addline('set %s=%s' % (key, value)) def unsetenv(self, key): diff --git a/src/rezplugins/shell/csh.py b/src/rezplugins/shell/csh.py index 3e50845a2..274bcb1f1 100644 --- a/src/rezplugins/shell/csh.py +++ b/src/rezplugins/shell/csh.py @@ -155,6 +155,7 @@ def _saferefenv(self, key): def setenv(self, key, value): value = self.escape_string(value) + value = self.normalize_if_path(key, value) self._addline('setenv %s %s' % (key, value)) def unsetenv(self, key): diff --git a/src/rezplugins/shell/sh.py b/src/rezplugins/shell/sh.py index df03dc996..73b126040 100644 --- a/src/rezplugins/shell/sh.py +++ b/src/rezplugins/shell/sh.py @@ -105,6 +105,7 @@ def _bind_interactive_rez(self): def setenv(self, key, value): value = self.escape_string(value) + value = self.normalize_if_path(key, value) self._addline('export %s=%s' % (key, value)) def unsetenv(self, key): From 1799574735d56a3372b45805a43ecb58111a8f2d Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Sat, 2 Apr 2022 12:17:40 +1100 Subject: [PATCH 2/8] added proper path normalization to windows shells --- src/rezplugins/shell/_utils/powershell_base.py | 10 ++++++++++ src/rezplugins/shell/cmd.py | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/src/rezplugins/shell/_utils/powershell_base.py b/src/rezplugins/shell/_utils/powershell_base.py index 9cc8bdaca..4c7365897 100644 --- a/src/rezplugins/shell/_utils/powershell_base.py +++ b/src/rezplugins/shell/_utils/powershell_base.py @@ -238,6 +238,16 @@ def escape_string(self, value): result += txt return result + def normalize_path(self, path): + """ + TODO: doesn't take into account escaped forward slashes, which would be + weird to have in a path, but is possible. + """ + if platform_.name == "windows": + return path.replace('/', '\\') + else: + return path + def _saferefenv(self, key): pass diff --git a/src/rezplugins/shell/cmd.py b/src/rezplugins/shell/cmd.py index 6ad083c78..e22b98efe 100644 --- a/src/rezplugins/shell/cmd.py +++ b/src/rezplugins/shell/cmd.py @@ -297,6 +297,13 @@ def escape_string(self, value): result += txt return result + def normalize_path(self, path): + """ + TODO: doesn't take into account escaped forward slashes, which would be + weird to have in a path, but is possible. + """ + return path.replace('/', '\\') + def _saferefenv(self, key): pass From fde5f31eebd7fa72516b811cdf8507186c0b60ed Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Tue, 5 Apr 2022 20:39:48 +1000 Subject: [PATCH 3/8] apply path normalization more broadly --- src/rez/resolved_context.py | 26 ++++++++++++------- src/rez/rex.py | 16 +++++++++--- src/rez/rex_bindings.py | 17 +++++++++--- src/rez/rezconfig.py | 2 +- .../shell/_utils/powershell_base.py | 7 ++--- src/rezplugins/shell/_utils/windows.py | 24 +++++++++++++++++ src/rezplugins/shell/cmd.py | 7 ++--- 7 files changed, 71 insertions(+), 28 deletions(-) create mode 100644 src/rezplugins/shell/_utils/windows.py diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index 3bd74498f..1de9c63ab 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -1948,19 +1948,25 @@ def _get_pre_resolve_bindings(self): @pool_memcached_connections def _execute(self, executor): - # bind various info to the execution context + """Bind various info to the execution context + """ + def normalized(path): + return executor.interpreter.normalize_path(path) + resolved_pkgs = self.resolved_packages or [] ephemerals = self.resolved_ephemerals or [] request_str = ' '.join(str(x) for x in self._package_requests) implicit_str = ' '.join(str(x) for x in self.implicit_packages) resolve_str = ' '.join(x.qualified_package_name for x in resolved_pkgs) - package_paths_str = os.pathsep.join(self.package_paths) req_timestamp_str = str(self.requested_timestamp or 0) + package_paths_str = os.pathsep.join( + normalized(x) for x in self.package_paths + ) header_comment(executor, "system setup") - executor.setenv("REZ_USED", self.rez_path) + executor.setenv("REZ_USED", normalized(self.rez_path)) executor.setenv("REZ_USED_VERSION", self.rez_version) executor.setenv("REZ_USED_TIMESTAMP", str(self.timestamp)) executor.setenv("REZ_USED_REQUESTED_TIMESTAMP", req_timestamp_str) @@ -1981,7 +1987,7 @@ def _execute(self, executor): not config.disable_rez_1_compatibility: request_str_ = " ".join([request_str, implicit_str]).strip() executor.setenv("REZ_VERSION", self.rez_version) - executor.setenv("REZ_PATH", self.rez_path) + executor.setenv("REZ_PATH", normalized(self.rez_path)) executor.setenv("REZ_REQUEST", request_str_) executor.setenv("REZ_RESOLVE", resolve_str) executor.setenv("REZ_RAW_REQUEST", request_str_) @@ -2005,7 +2011,9 @@ def _execute(self, executor): else: cached_root = None - variant_binding = VariantBinding(pkg, cached_root=cached_root) + variant_binding = VariantBinding( + pkg, cached_root=cached_root, interpreter=executor.interpreter + ) variant_bindings[pkg.name] = variant_binding # binds objects such as 'request', which are accessible before a resolve @@ -2038,7 +2046,7 @@ def _execute(self, executor): executor.setenv(prefix + "_MAJOR_VERSION", major_version) executor.setenv(prefix + "_MINOR_VERSION", minor_version) executor.setenv(prefix + "_PATCH_VERSION", patch_version) - executor.setenv(prefix + "_BASE", pkg.base) + executor.setenv(prefix + "_BASE", normalized(pkg.base)) variant_binding = variant_bindings[pkg.name] @@ -2046,9 +2054,9 @@ def _execute(self, executor): # set to cached payload rather than original executor.setenv(prefix + "_ROOT", variant_binding.root) # store extra var to indicate that root retarget occurred - executor.setenv(prefix + "_ORIG_ROOT", pkg.root) + executor.setenv(prefix + "_ORIG_ROOT", normalized(pkg.root)) else: - executor.setenv(prefix + "_ROOT", pkg.root) + executor.setenv(prefix + "_ROOT", normalized(pkg.root)) # package commands for attr in ("pre_commands", "commands", "post_commands"): @@ -2067,7 +2075,7 @@ def _execute(self, executor): executor.bind('this', variant_binding) executor.bind("version", VersionBinding(pkg.version)) executor.bind('root', variant_binding.root) - executor.bind('base', pkg.base) + executor.bind('base', normalized(pkg.base)) exc = None commands.set_package(pkg) diff --git a/src/rez/rex.py b/src/rez/rex.py index 8d549762d..5bb1b7c37 100644 --- a/src/rez/rex.py +++ b/src/rez/rex.py @@ -580,11 +580,16 @@ def normalize_path(self, path): return path def normalize_if_path(self, key, value): - """Normalize value if it's a path. + """Normalize value if it's a path(s). + + Note that `value` may be more than one os.pathsep-separated paths. """ if not any(fnmatch(key, x) for x in config.pathed_env_vars): return value - return self.normalize_path(value) + + paths = value.split(os.pathsep) + paths = [self.normalize_path(x) for x in paths] + return os.pathsep.join(paths) # --- internal commands, not exposed to public rex API @@ -1317,8 +1322,11 @@ def reset_globals(self): def append_system_paths(self): """Append system paths to $PATH.""" from rez.shells import Shell, create_shell - sh = self.interpreter if isinstance(self.interpreter, Shell) \ - else create_shell() + + if isinstance(self.interpreter, Shell): + sh = self.interpreter + else: + sh = create_shell() paths = sh.get_syspaths() paths_str = os.pathsep.join(paths) diff --git a/src/rez/rex_bindings.py b/src/rez/rex_bindings.py index 1805d6908..62b45acb6 100644 --- a/src/rez/rex_bindings.py +++ b/src/rez/rex_bindings.py @@ -19,7 +19,8 @@ class Binding(object): - """Abstract base class.""" + """Abstract base class. + """ def __init__(self, data=None): self._data = data or {} @@ -111,10 +112,13 @@ def __iter__(self): class VariantBinding(Binding): - """Binds a packages.Variant object.""" - def __init__(self, variant, cached_root=None): + """Binds a packages.Variant object. + """ + def __init__(self, variant, cached_root=None, interpreter=None): doc = dict(version=VersionBinding(variant.version)) super(VariantBinding, self).__init__(doc) + + self.__interpreter = interpreter self.__variant = variant self.__cached_root = cached_root @@ -125,7 +129,12 @@ def root(self): such as 'resolve.mypkg.root' resolve to the cached payload location, if the package is cached. """ - return self.__cached_root or self.__variant.root + root = self.__cached_root or self.__variant.root + + if self.__interpreter: + root = self.__interpreter.normalize_path(root) + + return root def __getattr__(self, attr): try: diff --git a/src/rez/rezconfig.py b/src/rez/rezconfig.py index d16625c98..42fd72adc 100644 --- a/src/rez/rezconfig.py +++ b/src/rez/rezconfig.py @@ -528,7 +528,7 @@ # because some shells need to apply path normalization. For example, the command # `env.PATH.append("{root}/bin")` will be normalized to (eg) `C:\...\bin` in a # `cmd` shell on Windows. Note that wildcards are supported. If this setting is -# not correctly configured, then your shell may not be guaranteed to work correctly. +# not correctly configured, then your shell may not work correctly. pathed_env_vars = [ "*PATH" ] diff --git a/src/rezplugins/shell/_utils/powershell_base.py b/src/rezplugins/shell/_utils/powershell_base.py index 4c7365897..9e6819c30 100644 --- a/src/rezplugins/shell/_utils/powershell_base.py +++ b/src/rezplugins/shell/_utils/powershell_base.py @@ -14,6 +14,7 @@ from rez.utils.platform_ import platform_ from rez.utils.execution import Popen from rez.util import shlex_join +from .windows import to_windows_path class PowerShellBase(Shell): @@ -239,12 +240,8 @@ def escape_string(self, value): return result def normalize_path(self, path): - """ - TODO: doesn't take into account escaped forward slashes, which would be - weird to have in a path, but is possible. - """ if platform_.name == "windows": - return path.replace('/', '\\') + return to_windows_path(path) else: return path diff --git a/src/rezplugins/shell/_utils/windows.py b/src/rezplugins/shell/_utils/windows.py new file mode 100644 index 000000000..6d7d344e4 --- /dev/null +++ b/src/rezplugins/shell/_utils/windows.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Contributors to the Rez Project + +import re + + +def to_posix_path(path): + """Convert (eg) "C:\foo" to "/c/foo" + TODO: doesn't take into account escaped bask slashes, which would be + weird to have in a path, but is possible. + """ + if re.match("[A-Z]:", path): + path = '/' + path[0].lower() + path[2:] + return path.replace('\\', '/') + + +def to_windows_path(path): + """Convert (eg) "C:\foo/bin" to "C:\foo\bin" + The mixed syntax results from strings in package commands such as + "{root}/bin" being interpreted in a windows shell. + TODO: doesn't take into account escaped forward slashes, which would be + weird to have in a path, but is possible. + """ + return path.replace('/', '\\') diff --git a/src/rezplugins/shell/cmd.py b/src/rezplugins/shell/cmd.py index e22b98efe..09661dde5 100644 --- a/src/rezplugins/shell/cmd.py +++ b/src/rezplugins/shell/cmd.py @@ -12,6 +12,7 @@ from rez.utils.execution import Popen from rez.utils.platform_ import platform_ from rez.vendor.six import six +from ._utils.windows import to_windows_path from functools import partial import os import re @@ -298,11 +299,7 @@ def escape_string(self, value): return result def normalize_path(self, path): - """ - TODO: doesn't take into account escaped forward slashes, which would be - weird to have in a path, but is possible. - """ - return path.replace('/', '\\') + return to_windows_path(path) def _saferefenv(self, key): pass From 5377bebf4286e25614fefa671949dd84cebd32f9 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Tue, 5 Apr 2022 20:42:09 +1000 Subject: [PATCH 4/8] hmm, very strict copyright checker --- src/rezplugins/shell/_utils/windows.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rezplugins/shell/_utils/windows.py b/src/rezplugins/shell/_utils/windows.py index 6d7d344e4..6ef1004b7 100644 --- a/src/rezplugins/shell/_utils/windows.py +++ b/src/rezplugins/shell/_utils/windows.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright Contributors to the Rez Project + import re From 8c7fa39ddcf2efdcae0e41fb9d7a536a44daa38d Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Thu, 7 Apr 2022 20:11:04 +1000 Subject: [PATCH 5/8] completed further cases requiring path normalization --- src/rez/build_system.py | 44 ++++++++++------ src/rez/rex.py | 52 ++++++++++++------- src/rez/tests/test_shells.py | 4 +- src/rezplugins/build_system/custom.py | 39 ++++++++------ .../shell/_utils/powershell_base.py | 18 +++---- src/rezplugins/shell/_utils/windows.py | 25 +++++++-- src/rezplugins/shell/cmd.py | 8 +-- src/rezplugins/shell/csh.py | 8 +-- src/rezplugins/shell/sh.py | 8 +-- src/rezplugins/shell/tcsh.py | 5 +- 10 files changed, 139 insertions(+), 72 deletions(-) diff --git a/src/rez/build_system.py b/src/rez/build_system.py index 28906c758..1ab9a727a 100644 --- a/src/rez/build_system.py +++ b/src/rez/build_system.py @@ -7,6 +7,7 @@ from rez.build_process import BuildType from rez.exceptions import BuildSystemError from rez.packages import get_developer_package +from rez.rex_bindings import VariantBinding def get_buildsys_types(): @@ -234,10 +235,9 @@ def build(self, context, variant, build_path, install_path, install=False, raise NotImplementedError @classmethod - def get_standard_vars(cls, context, variant, build_type, install, + def set_standard_vars(cls, executor, context, variant, build_type, install, build_path, install_path=None): - """Returns a standard set of environment variables that can be set - for the build system to use + """Set some standard env vars that all build systems can rely on. """ from rez.config import config @@ -251,16 +251,18 @@ def get_standard_vars(cls, context, variant, build_type, install, vars_ = { 'REZ_BUILD_ENV': 1, - 'REZ_BUILD_PATH': build_path, + 'REZ_BUILD_PATH': executor.normalize_path(build_path), 'REZ_BUILD_THREAD_COUNT': package.config.build_thread_count, 'REZ_BUILD_VARIANT_INDEX': variant.index or 0, 'REZ_BUILD_VARIANT_REQUIRES': ' '.join(variant_requires), - 'REZ_BUILD_VARIANT_SUBPATH': variant_subpath, + 'REZ_BUILD_VARIANT_SUBPATH': executor.normalize_path(variant_subpath), 'REZ_BUILD_PROJECT_VERSION': str(package.version), 'REZ_BUILD_PROJECT_NAME': package.name, 'REZ_BUILD_PROJECT_DESCRIPTION': (package.description or '').strip(), 'REZ_BUILD_PROJECT_FILE': package.filepath, - 'REZ_BUILD_SOURCE_PATH': os.path.dirname(package.filepath), + 'REZ_BUILD_SOURCE_PATH': executor.normalize_path( + os.path.dirname(package.filepath) + ), 'REZ_BUILD_REQUIRES': ' '.join( str(x) for x in context.requested_packages(True) ), @@ -272,14 +274,16 @@ def get_standard_vars(cls, context, variant, build_type, install, } if install_path: - vars_['REZ_BUILD_INSTALL_PATH'] = install_path + vars_['REZ_BUILD_INSTALL_PATH'] = executor.normalize_path(install_path) if config.rez_1_environment_variables and \ not config.disable_rez_1_compatibility and \ build_type == BuildType.central: vars_['REZ_IN_REZ_RELEASE'] = 1 - return vars_ + # set env vars + for key, value in vars_.items(): + executor.env[key] = value @classmethod def add_pre_build_commands(cls, executor, variant, build_type, install, @@ -292,16 +296,29 @@ def add_pre_build_commands(cls, executor, variant, build_type, install, build_ns = { "build_type": build_type.name, "install": install, - "build_path": build_path, - "install_path": install_path + "build_path": executor.normalize_path(build_path), + "install_path": executor.normalize_path(install_path) } # execute pre_build_commands() + # note that we need to wrap variant in a VariantBinding so that any refs + # to (eg) 'this.root' in pre_build_commands() will get the possibly + # normalized path. + # pre_build_commands = getattr(variant, "pre_build_commands") + # TODO I suspect variant root isn't correctly set to the cached root + # when pkg caching is enabled (see use of VariantBinding in + # ResolvedContext._execute). + # + bound_variant = VariantBinding( + variant, + interpreter=executor.interpreter + ) + if pre_build_commands: with executor.reset_globals(): - executor.bind("this", variant) + executor.bind("this", bound_variant) executor.bind("build", ROA(build_ns)) executor.execute_code(pre_build_commands) @@ -311,7 +328,7 @@ def add_standard_build_actions(cls, executor, context, variant, build_type, """Perform build actions common to every build system. """ # set env vars - env_vars = cls.get_standard_vars( + cls.set_standard_vars( context=context, variant=variant, build_type=build_type, @@ -319,6 +336,3 @@ def add_standard_build_actions(cls, executor, context, variant, build_type, build_path=build_path, install_path=install_path ) - - for var, value in env_vars.items(): - executor.env[var] = value diff --git a/src/rez/rex.py b/src/rez/rex.py index 5bb1b7c37..0a820bdb4 100644 --- a/src/rez/rex.py +++ b/src/rez/rex.py @@ -233,7 +233,7 @@ def get_public_methods(self): ('undefined', self.undefined)] def _env_sep(self, name): - return self._env_sep_map.get(name, os.pathsep) + return self._env_sep_map.get(name, self.interpreter.pathsep) def _is_verbose(self, command): if isinstance(self.verbose, (list, tuple)): @@ -473,6 +473,11 @@ class ActionInterpreter(object): """ expand_env_vars = False + # Path separator. There are cases (eg gitbash - git for windows) where the + # path separator does not match the system (ie os.pathsep) + # + pathsep = os.pathsep + # RegEx that captures environment variables (generic form). # Extend/override to regex formats that can capture environment formats # in other interpreters like shells if needed @@ -538,34 +543,42 @@ def shebang(self): # --- other - def escape_string(self, value): + def escape_string(self, value, is_path=False): """Escape a string. Escape the given string so that special characters (such as quotes and whitespace) are treated properly. If `value` is a string, assume that this is an expandable string in this interpreter. + Note that `is_path` provided because of the special case where a + path-like envvar is set. In this case, path normalization, if it needs + to occur, has to be part of the string escaping process. + Note: This default implementation returns the string with no escaping applied. Args: value (str or `EscapedString`): String to escape. + is_path (bool): True if the value is path-like. Returns: str: The escaped string. """ return str(value) + @classmethod + def _is_pathed_key(cls, key): + return any(fnmatch(key, x) for x in config.pathed_env_vars) + def normalize_path(self, path): """Normalize a path. Change `path` to a valid filepath representation for this interpreter. - Note: - Assume that `path` has already been escaped at this point. This will - matter for edge cases, like a path on linux containing an escaped - back slash. + IMPORTANT: Because var references like ${THIS} might be passed to funcs + like appendvar, `path` might be in this form. You need to take that + into account (ie, ensure normalization doesn't break such a var reference). Args: path (str): A filepath which may be in posix format, or windows @@ -579,17 +592,13 @@ def normalize_path(self, path): """ return path - def normalize_if_path(self, key, value): + def normalize_paths(self, value): """Normalize value if it's a path(s). - - Note that `value` may be more than one os.pathsep-separated paths. + Note that `value` may be more than one pathsep-delimited paths. """ - if not any(fnmatch(key, x) for x in config.pathed_env_vars): - return value - - paths = value.split(os.pathsep) + paths = value.split(self.pathsep) paths = [self.normalize_path(x) for x in paths] - return os.pathsep.join(paths) + return self.pathsep.join(paths) # --- internal commands, not exposed to public rex API @@ -658,7 +667,7 @@ def setenv(self, key, value): if self.update_session: if key == 'PYTHONPATH': value = self.escape_string(value) - sys.path = value.split(os.pathsep) + sys.path = value.split(self.pathsep) def unsetenv(self, key): pass @@ -1328,9 +1337,8 @@ def append_system_paths(self): else: sh = create_shell() - paths = sh.get_syspaths() - paths_str = os.pathsep.join(paths) - self.env.PATH.append(paths_str) + for path in sh.get_syspaths(): + self.env.PATH.append(path) def prepend_rez_path(self): """Prepend rez path to $PATH.""" @@ -1342,6 +1350,14 @@ def append_rez_path(self): if system.rez_bin_path: self.env.PATH.append(system.rez_bin_path) + def normalize_path(self, path): + """Normalize a path. + Note that in many interpreters this will be unchanged. + Returns: + str: The normalized path. + """ + return self.interpreter.normalize_path(path) + @classmethod def compile_code(cls, code, filename=None, exec_namespace=None): """Compile and possibly execute rex code. diff --git a/src/rez/tests/test_shells.py b/src/rez/tests/test_shells.py index fb67337f5..24b86913f 100644 --- a/src/rez/tests/test_shells.py +++ b/src/rez/tests/test_shells.py @@ -380,8 +380,8 @@ def _rex_appending(): expected_output = [ "hey", - os.pathsep.join(["hey", "$DAVE"]), - os.pathsep.join(["hey", "$DAVE", "Dave's not here man"]) + sh.pathsep.join(["hey", "$DAVE"]), + sh.pathsep.join(["hey", "$DAVE", "Dave's not here man"]) ] _execute_code(_rex_appending, expected_output) diff --git a/src/rezplugins/build_system/custom.py b/src/rezplugins/build_system/custom.py index 7fb67f98e..165a10423 100644 --- a/src/rezplugins/build_system/custom.py +++ b/src/rezplugins/build_system/custom.py @@ -21,6 +21,7 @@ from rez.utils.execution import create_forwarding_script from rez.packages import get_developer_package from rez.resolved_context import ResolvedContext +from rez.shells import create_shell from rez.exceptions import PackageMetadataError from rez.utils.colorize import heading, Printer from rez.utils.logging_ import print_warning @@ -117,14 +118,16 @@ def build(self, context, variant, build_path, install_path, install=False, if self.write_build_scripts: # write out the script that places the user in a build env build_env_script = os.path.join(build_path, "build-env") - create_forwarding_script(build_env_script, - module=("build_system", "custom"), - func_name="_FWD__spawn_build_shell", - working_dir=self.working_dir, - build_path=build_path, - variant_index=variant.index, - install=install, - install_path=install_path) + create_forwarding_script( + build_env_script, + module=("build_system", "custom"), + func_name="_FWD__spawn_build_shell", + working_dir=self.working_dir, + build_path=build_path, + variant_index=variant.index, + install=install, + install_path=install_path + ) ret["success"] = True ret["build_env_script"] = build_env_script @@ -139,13 +142,19 @@ def build(self, context, variant, build_path, install_path, install=False, return ret def expand(txt): - return txt.format(build_path=build_path, - install="install" if install else '', - install_path=install_path, - name=self.package.name, - root=self.package.root, - variant_index=variant.index if variant.index is not None else '', - version=self.package.version).strip() + # we need a shell here because build_command may reference vars like + # {root}, and that may require path normalization. + sh = create_shell() + + return txt.format( + build_path=sh.normalize_path(build_path), + install_path=sh.normalize_path(install_path), + root=sh.normalize_path(self.package.root), + install="install" if install else '', + name=self.package.name, + variant_index=variant.index if variant.index is not None else '', + version=self.package.version + ).strip() if isinstance(command, basestring): if self.build_args: diff --git a/src/rezplugins/shell/_utils/powershell_base.py b/src/rezplugins/shell/_utils/powershell_base.py index 9e6819c30..ca4171693 100644 --- a/src/rezplugins/shell/_utils/powershell_base.py +++ b/src/rezplugins/shell/_utils/powershell_base.py @@ -226,7 +226,7 @@ def get_output(self, style=OutputStyle.file): script = '&& '.join(lines) return script - def escape_string(self, value): + def escape_string(self, value, is_path=False): value = EscapedString.promote(value) value = value.expanduser() result = '' @@ -235,6 +235,9 @@ def escape_string(self, value): if is_literal: txt = self._escape_quotes(self._escape_vars(txt)) else: + if is_path: + txt = self.normalize_paths(txt) + txt = self._escape_quotes(txt) result += txt return result @@ -252,30 +255,27 @@ def shebang(self): pass def setenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) self._addline('Set-Item -Path "Env:{0}" -Value "{1}"'.format(key, value)) def prependenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) # 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( 'Set-Item -Path "Env:{0}" -Value ("{1}{2}" + (Get-ChildItem "Env:{0}").Value)'.format( - key, value, os.path.pathsep) + key, value, self.pathsep) ) def appendenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) # 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( 'Set-Item -Path "Env:{0}" -Value ((Get-ChildItem "Env:{0}").Value + "{1}{2}")'.format( - key, os.path.pathsep, value) + key, self.pathsep, value) ) def unsetenv(self, key): diff --git a/src/rezplugins/shell/_utils/windows.py b/src/rezplugins/shell/_utils/windows.py index 6ef1004b7..ce43e1567 100644 --- a/src/rezplugins/shell/_utils/windows.py +++ b/src/rezplugins/shell/_utils/windows.py @@ -2,23 +2,42 @@ # Copyright Contributors to the Rez Project +import os import re +_drive_start_regex = re.compile(r"^([A-Za-z]):\\") +_env_var_regex = re.compile(r"%([^%]*)%") + + def to_posix_path(path): """Convert (eg) "C:\foo" to "/c/foo" TODO: doesn't take into account escaped bask slashes, which would be weird to have in a path, but is possible. """ - if re.match("[A-Z]:", path): - path = '/' + path[0].lower() + path[2:] - return path.replace('\\', '/') + + # expand refs like %SYSTEMROOT%, leave as-is if not in environ + def _repl(m): + varname = m.groups()[0] + return os.getenv(varname, m.group()) + + path = _env_var_regex.sub(_repl, path) + + # C:\ ==> /C/ + path = _drive_start_regex.sub("/\\1/", path) + + # backslash ==> fwdslash + path = path.replace('\\', '/') + + return path def to_windows_path(path): """Convert (eg) "C:\foo/bin" to "C:\foo\bin" + The mixed syntax results from strings in package commands such as "{root}/bin" being interpreted in a windows shell. + TODO: doesn't take into account escaped forward slashes, which would be weird to have in a path, but is possible. """ diff --git a/src/rezplugins/shell/cmd.py b/src/rezplugins/shell/cmd.py index 09661dde5..d3b80f6b6 100644 --- a/src/rezplugins/shell/cmd.py +++ b/src/rezplugins/shell/cmd.py @@ -274,7 +274,7 @@ def get_output(self, style=OutputStyle.file): script = '&& '.join(lines) return script - def escape_string(self, value): + def escape_string(self, value, is_path=False): """Escape the <, >, ^, and & special characters reserved by Windows. Args: @@ -294,6 +294,9 @@ def escape_string(self, value): # Note that cmd uses ^% while batch files use %% to escape % txt = self._env_var_regex.sub(r"%%\1%%", txt) else: + if is_path: + txt = self.normalize_paths(txt) + txt = self._escaper(txt) result += txt return result @@ -308,8 +311,7 @@ def shebang(self): pass def setenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) self._addline('set %s=%s' % (key, value)) def unsetenv(self, key): diff --git a/src/rezplugins/shell/csh.py b/src/rezplugins/shell/csh.py index 274bcb1f1..4a80e0d9e 100644 --- a/src/rezplugins/shell/csh.py +++ b/src/rezplugins/shell/csh.py @@ -98,7 +98,7 @@ def get_startup_sequence(cls, rcfile, norc, stdin, command): source_bind_files=(not norc) ) - def escape_string(self, value): + def escape_string(self, value, is_path=False): value = EscapedString.promote(value) value = value.expanduser() result = '' @@ -109,6 +109,9 @@ def escape_string(self, value): if not txt.startswith("'"): txt = "'%s'" % txt else: + if is_path: + txt = self.normalize_paths(txt) + txt = txt.replace('"', '"\\""') txt = txt.replace('!', '\\!') txt = '"%s"' % txt @@ -154,8 +157,7 @@ def _saferefenv(self, key): self._addline("if (!($?%s)) setenv %s" % (key, key)) def setenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) self._addline('setenv %s %s' % (key, value)) def unsetenv(self, key): diff --git a/src/rezplugins/shell/sh.py b/src/rezplugins/shell/sh.py index 73b126040..7fbade028 100644 --- a/src/rezplugins/shell/sh.py +++ b/src/rezplugins/shell/sh.py @@ -104,8 +104,7 @@ def _bind_interactive_rez(self): self._addline(cmd % r"\[\e[1m\]$REZ_ENV_PROMPT\[\e[0m\]") def setenv(self, key, value): - value = self.escape_string(value) - value = self.normalize_if_path(key, value) + value = self.escape_string(value, is_path=self._is_pathed_key(key)) self._addline('export %s=%s' % (key, value)) def unsetenv(self, key): @@ -120,7 +119,7 @@ def source(self, value): value = self.escape_string(value) self._addline('. %s' % value) - def escape_string(self, value): + def escape_string(self, value, is_path=False): value = EscapedString.promote(value) value = value.expanduser() result = '' @@ -131,6 +130,9 @@ def escape_string(self, value): if not txt.startswith("'"): txt = "'%s'" % txt else: + if is_path: + txt = self.normalize_paths(txt) + txt = txt.replace('\\', '\\\\') txt = txt.replace('"', '\\"') txt = '"%s"' % txt diff --git a/src/rezplugins/shell/tcsh.py b/src/rezplugins/shell/tcsh.py index 62f60226d..da5f1c27c 100644 --- a/src/rezplugins/shell/tcsh.py +++ b/src/rezplugins/shell/tcsh.py @@ -19,7 +19,7 @@ class TCSH(CSH): def name(cls): return 'tcsh' - def escape_string(self, value): + def escape_string(self, value, is_path=False): value = EscapedString.promote(value) value = value.expanduser() result = '' @@ -30,6 +30,9 @@ def escape_string(self, value): if not txt.startswith("'"): txt = "'%s'" % txt else: + if is_path: + txt = self.normalize_paths(txt) + txt = txt.replace('"', '"\\""') txt = txt.replace('!', '\\!') txt = '"%s"' % txt From c2c0fda14089d531e6c5d5f68bee5e88da9ef6c5 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Thu, 7 Apr 2022 20:13:38 +1000 Subject: [PATCH 6/8] missing executor arg --- src/rez/build_system.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rez/build_system.py b/src/rez/build_system.py index 1ab9a727a..d0c5f90fc 100644 --- a/src/rez/build_system.py +++ b/src/rez/build_system.py @@ -329,6 +329,7 @@ def add_standard_build_actions(cls, executor, context, variant, build_type, """ # set env vars cls.set_standard_vars( + executor=executor, context=context, variant=variant, build_type=build_type, From f90f3ccd7e6a65c0e5417e2f67fa6f31bfd21464 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Thu, 7 Apr 2022 20:50:17 +1000 Subject: [PATCH 7/8] added wiki entry on filepaths --- wiki/pages/Package-Commands.md | 27 +++++++++++++++++++++++--- wiki/pages/Package-Definition-Guide.md | 14 ++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/wiki/pages/Package-Commands.md b/wiki/pages/Package-Commands.md index e0aec5369..79aa1618a 100644 --- a/wiki/pages/Package-Commands.md +++ b/wiki/pages/Package-Commands.md @@ -138,6 +138,27 @@ env.FOO = literal("${USER}") | export FOO='${USER}' env.FOO = expandable("${USER}") | export FOO="${USER}" env.FOO = expandvars("${USER}") | export FOO="jbloggs" +## Filepaths + +Rez expects POSIX-style filepath syntax in package commands, regardless of the shell or platform. +Thus, even if you're on Windows, you should do this: + + def commands(): + env.PATH.append("{root}/bin") # note the forward slash + +Where necessary, filepaths will be automatically normalized for you - that is, converted into +the syntax expected by the shell. In order for this to work correctly however, rez needs to know +what environment variables are actually paths. You determine this with the +[pathed_env_vars](Configuring-Rez#pathed_env_vars) config setting. By default, any environment +variable ending in `PATH` will be treated as a filepath or list of filepaths, and any +set/append/prepend operation on it will cause those values to be path-normalized automatically. + +> [[media/icons/warning.png]] Avoid using `os.pathsep` or hardcoded lists of paths such as +> `{root}/foo:{root}/bah`. Doing so can cause your package to be incompatible with some shells or +> platforms. Even the seemingly innocuous `os.pathsep` is an issue, because there are some cases +> (eg Git for Windows, aka git-bash) where the shell's path separator does not match the underlying +> system's. + ## Pre And Post Commands Occasionally it's useful for a package to run commands either before or after all other packages, @@ -371,7 +392,7 @@ a request for `.foo.cli-0`: Prints to standard error. > [[media/icons/info.png]] This function just prints the error, it does not prevent the target -environment from being constructed (use the [stop](#stop) command for that). +> environment from being constructed (use the [stop](#stop) command for that). ### getenv *Function* @@ -460,8 +481,8 @@ Use `get_range` to test with the [intersects](Package-Commands#intersects) funct info("maya 2019.* was asked for!") > [[media/icons/info.png]] If multiple requests are present that refer to the same package, the -request is combined ahead of time. In other words, if requests *foo-4+* and *foo-<6* were both -present, the single request *foo-4+<6* would be present in the *request* object. +> request is combined ahead of time. In other words, if requests *foo-4+* and *foo-<6* were both +> present, the single request *foo-4+<6* would be present in the *request* object. ### resolve *Dict-like object* diff --git a/wiki/pages/Package-Definition-Guide.md b/wiki/pages/Package-Definition-Guide.md index 0815069ef..ae4a76fa2 100644 --- a/wiki/pages/Package-Definition-Guide.md +++ b/wiki/pages/Package-Definition-Guide.md @@ -55,7 +55,7 @@ Python variables that do **not** become package attributes include: * Any variable with a leading double underscore; * Any variable that is a [build-time package attribute](#build-time-package-attributes). -## Package Attributes As Functions +### Function Attributes Package attributes can be implemented as functions - the return value of the function becomes the attribute value. There are two types of attribute functions - *early binding* functions, @@ -65,7 +65,7 @@ and *late binding* functions - and these are decorated using *@early* and *@late > late bound, but are not the same as a standard function attribute, and are *never* decorated > with the early or late decorators. -### Early Binding Functions +#### Early Binding Functions Early binding functions use the *@early* decorator. They are evaluated at *build time*, hence the 'early' in 'early binding'. Any package attribute can be implemented as an early binding function. @@ -108,7 +108,7 @@ define an arbitrary function earlier in the python source. You can always use a two as well - an early binding function can call an arbitrary function defined at the bottom of your definition file. -#### Available Objects +##### Available Objects Following is the list of objects that are available during early evaluation. @@ -140,7 +140,7 @@ might look like so: > [[media/icons/warning.png]] You **must** ensure that your early-bound function returns the value > you want to see in the installed package, when `building` is False. -### Late Binding Functions +#### Late Binding Functions Late binding functions stay as functions in the installed package definition, and are only evaluated lazily, when the attribute is accessed for the first time (the return value is then cached). @@ -202,7 +202,7 @@ Note how in the *_tools* function we're referring to a relative path. Remember t functions are evaluated at build time - the package hasn't actually been built or installed yet, so attributes such as *this.root* don't exist. -#### The *in_context* Function +##### The *in_context* Function When late binding functions are evaluated, a boolean function *in_context* is present, which returns True if the package is part of a resolved context, or False otherwise. For example, @@ -230,7 +230,7 @@ current env; if it was, a maya-specific tool *maya-edit* is added to the tool li > value regardless of whether *in_context* is True or False. Otherwise, simply trying to > query the package attributes (using *rez-search* for example) may cause errors. -#### Available Objects +##### Available Objects Following is the list of objects that are available during late evaluation, if *in_context* is *True*: @@ -254,7 +254,7 @@ The following objects are available in *all* cases: > attributes that packages don't - notably, *root* and *index*. Use the properties > `this.is_package` and `this.is_variant` to distinguish the case if needed. -#### Example - Late Bound build_requires +##### Example - Late Bound build_requires Here is an example of a package.py with a late-bound `build_requires` field: From cee67d72c97e43dc23e665f8ab4f5a297cf51655 Mon Sep 17 00:00:00 2001 From: nerdvegas Date: Thu, 7 Apr 2022 21:19:41 +1000 Subject: [PATCH 8/8] fixed some missing cherrypicked changes --- src/rez/resolved_context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rez/resolved_context.py b/src/rez/resolved_context.py index 1de9c63ab..e098f1b06 100644 --- a/src/rez/resolved_context.py +++ b/src/rez/resolved_context.py @@ -1951,7 +1951,7 @@ def _execute(self, executor): """Bind various info to the execution context """ def normalized(path): - return executor.interpreter.normalize_path(path) + return executor.normalize_path(path) resolved_pkgs = self.resolved_packages or [] ephemerals = self.resolved_ephemerals or [] @@ -1960,7 +1960,7 @@ def normalized(path): implicit_str = ' '.join(str(x) for x in self.implicit_packages) resolve_str = ' '.join(x.qualified_package_name for x in resolved_pkgs) req_timestamp_str = str(self.requested_timestamp or 0) - package_paths_str = os.pathsep.join( + package_paths_str = executor.interpreter.pathsep.join( normalized(x) for x in self.package_paths )