Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Combined packages (multi packages) fail on windows #1758

Open
cfxegbert opened this issue May 17, 2024 · 1 comment · May be fixed by #1759
Open

Combined packages (multi packages) fail on windows #1758

cfxegbert opened this issue May 17, 2024 · 1 comment · May be fixed by #1759
Assignees
@maxnbk
Labels
bug os:windows Windows-specific rez-env

Comments

@cfxegbert
Copy link
Contributor

A simple combined package combined_test.py

name = "combined_test"
versions = ["1.0"]

def commands():
    info("testing")

On a non-windows environment

> rez-env combined_test                                                                                                                                                                                                                                                                                          
testing

You are now in a rez-configured environment.

resolved by rminsk@LMR007X6202HOME, on Fri May 17 11:40:56 2024, using Rez v3.1.0

requested packages:
combined_test
~platform==osx  (implicit)
~arch==arm64    (implicit)
~os==osx-13     (implicit)

resolved packages:
combined_test-1.0  /Users/rminsk/packages/combined_test.py<1.0>[]  (local)

> env | grep REZ_COMBINED_TEST | sort                                                                                                                                                                                                                                                                            
REZ_COMBINED_TEST_BASE=None
REZ_COMBINED_TEST_MAJOR_VERSION=1
REZ_COMBINED_TEST_MINOR_VERSION=0
REZ_COMBINED_TEST_PATCH_VERSION=
REZ_COMBINED_TEST_ROOT=None
REZ_COMBINED_TEST_VERSION=1.0

Notice that the base and root are set to None on a combined package.

On a windows environment:

$ rez-env combined_test
Traceback (most recent call last):
  File "C:\Program Files\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\1minsr002\rez\Scripts\rez\rez-env.exe\__main__.py", line 7, in <module>
  File "c:\users\1minsr002\rez\lib\site-packages\rez\cli\_entry_points.py", line 148, in run_rez_env
    return run("env")
  File "c:\users\1minsr002\rez\lib\site-packages\rez\cli\_main.py", line 189, in run
    returncode = run_cmd()
  File "c:\users\1minsr002\rez\lib\site-packages\rez\cli\_main.py", line 181, in run_cmd
    return func(opts, opts.parser, extra_arg_groups)
  File "c:\users\1minsr002\rez\lib\site-packages\rez\cli\env.py", line 253, in command
    returncode, _, _ = context.execute_shell(
  File "c:\users\1minsr002\rez\lib\site-packages\rez\resolved_context.py", line 1045, in _check
    return fn(self, *nargs, **kwargs)
  File "c:\users\1minsr002\rez\lib\site-packages\rez\resolved_context.py", line 1437, in execute_shell
    self._execute(executor)
  File "c:\users\1minsr002\rez\lib\site-packages\rez\utils\memcached.py", line 262, in wrapper
    return func(*nargs, **kwargs)
  File "c:\users\1minsr002\rez\lib\site-packages\rez\resolved_context.py", line 2060, in _execute
    executor.setenv(prefix + "_BASE", normalized(pkg.base))
  File "c:\users\1minsr002\rez\lib\site-packages\rez\resolved_context.py", line 1961, in normalized
    return executor.normalize_path(path)
  File "c:\users\1minsr002\rez\lib\site-packages\rez\rex.py", line 1349, in normalize_path
    return self.interpreter.normalize_path(path)
  File "c:\users\1minsr002\rez\lib\site-packages\rezplugins\shell\_utils\powershell_base.py", line 252, in normalize_path
    return to_windows_path(path)
  File "c:\users\1minsr002\rez\lib\site-packages\rezplugins\shell\_utils\windows.py", line 47, in to_windows_path
    return path.replace('/', '\\')
AttributeError: 'NoneType' object has no attribute 'replace'

This is coming from setting the _BASE and _ROOT environment variables
https://github.com/AcademySoftwareFoundation/rez/blob/main/src/rez/resolved_context.py#L2060-L2070
The normalize function on non-windows environments just returns the passed in variable. On windows it tries to convert drive letters to mount points (gitbash) and "/" to "\" (gitbash and powershell) assuming the value passed in is a string. On non-windows environments the None value is converted to string in setenv when the value is eventually passed to ActionManager._format where if the value is not a str or EscapedString it is converted to string.
https://github.com/AcademySoftwareFoundation/rez/blob/main/src/rez/rex.py#L233-L238

I have thought of three different solutions:

  1. Change the the normalized function to do a None check and return the string "None." This would be backwards compatible with the current non-windows behavior.
  2. If pkg.base/pkg.root is None then set the environment variable to the empty string. This would set an empty environment on non-windows environment and unset the environment variable on windows environments. This seems the least desirable because of the different behavior between windows and non-windows.
  3. If pkg.base/pkg.root is None then do not set the _BASE and _ROOT environment variables. This consistent across platforms but is a change of behavior for non-windows environments.

I have implemented solution 1 and will be submitting a pull request shortly.

Environment

  • OS Windows
  • Rez version 3.1.1
  • Rez python version 3.10.10
@cfxegbert cfxegbert added the bug label May 17, 2024
@cfxegbert cfxegbert linked a pull request May 17, 2024 that will close this issue
Open
@maxnbk maxnbk self-assigned this Sep 19, 2024
@maxnbk
Copy link
Contributor

maxnbk commented Sep 19, 2024

assigning myself to remind myself to dig into the origins and intent of this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maxnbk maxnbk

Labels
bug os:windows Windows-specific rez-env
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix for combined packages on windows cfxegbert/rez
3 participants
@maxnbk @JeanChristopheMorinPerso @cfxegbert