Rez pip_install_remaps need to escape sep
The pip_install_remaps feature works great for things on linux, but I'm running into an issue on windows that seems to be due to the backslash directory separator. When you rez-pip install networkx, it comes with a bunch of docs and examples. I've remapped them to copy to a "docs" directory next to the "python" directory. However, this doesn't work on windows due to os.sep being confused for a matching group in regex.
Environment
- OS windows 10
- Rez version 2.111.3
- Rez python version 3.9
To Reproduce
- Make sure the rezconfig.py file has a pip_install_remaps with:
{
"record_path": r"{pardir}{sep}{pardir}{sep}share{sep}doc{sep}networkx-([\w\.]+){sep}(.*)",
"pip_install": r"share{sep}doc{sep}networkx-\1{sep}\2",
"rez_install": r"doc{sep}\2",
},
-
rez-pip -i networkx
Expected behavior networkx should install and the docs should be remapped
Actual behavior An error occurs during the install process.
Traceback (most recent call last):
File "C:\Program Files\Python39\lib\sre_parse.py", line 1051, in parse_template
this = chr(ESCAPES[this][1])
KeyError: '\\d'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "C:\Program Files\Python39\lib\runpy.py", line 197, in _run_module_as_main
return _run_code(code, main_globals, None,
File "C:\Program Files\Python39\lib\runpy.py", line 87, in _run_code
exec(code, run_globals)
File "C:\rez\Scripts\rez\rez-pip.exe\__main__.py", line 7, in <module>
File "c:\rez\lib\site-packages\rez\cli\_entry_points.py", line 183, in run_rez_pip
return run("pip")
File "c:\rez\lib\site-packages\rez\cli\_main.py", line 191, in run
returncode = run_cmd()
File "c:\rez\lib\site-packages\rez\cli\_main.py", line 183, in run_cmd
return func(opts, opts.parser, extra_arg_groups)
File "c:\rez\lib\site-packages\rez\cli\pip.py", line 67, in command
pip_install_package(
File "c:\rez\lib\site-packages\rez\pip.py", line 366, in pip_install_package
src_dst_lut = _get_distribution_files_mapping(distribution, targetpath)
File "c:\rez\lib\site-packages\rez\pip.py", line 565, in _get_distribution_files_mapping
rel_src, rel_dest = get_mapping(rel_src_orig)
File "c:\rez\lib\site-packages\rez\pip.py", line 515, in get_mapping
pip_subpath = re.sub(path, remap['pip_install'], rel_src)
File "C:\Program Files\Python39\lib\re.py", line 210, in sub
return _compile(pattern, flags).sub(repl, string, count)
File "C:\Program Files\Python39\lib\re.py", line 327, in _subx
template = _compile_repl(template, pattern)
File "C:\Program Files\Python39\lib\re.py", line 318, in _compile_repl
return sre_parse.parse_template(repl, pattern)
File "C:\Program Files\Python39\lib\sre_parse.py", line 1054, in parse_template
raise s.error('bad escape %s' % this, len(this))
re.error: bad escape \d at position 5
In rez/pip.py above line 515, I added extra logging to see what path, remap['pip_install'], and rel_src are:
17:20:33 DEBUG path: \.\.\\\.\.\\share\\doc\\networkx-([\w\.]+)\\(.*)
17:20:33 DEBUG pip_install: share\doc\networkx-\1\\2
17:20:33 DEBUG rel_src: ..\..\share\doc\networkx-2.8.8\LICENSE.txt
Notice on the 2nd line, the backslashes are not escaped, so regex thinks that \d is a matching group.
As a test, I opened up rez/config.py and changed line 143 to:
key: expression.format(**(self.RE_TOKENS))
That way the pip_install and rez_install values in the pip_install_remaps have their os.sep escaped and the remap works.
Thanks for the report. As a side note: I am not a fan of how pip_install_remap works now as it forces us to add package specific (or better: files in packages specific) configuration to global config. It is also quite a lot of hassle everytime we encounter a package that requires it. I would rather have sensible defaults for files with unknown locations, output a warning and make pip_install_remap optional for those shops (or packages) that require it.
Sorry if that seemed like rambling, I was trying to type it all up before the end of the work day.
I agree, it's a bit confusing and (in this case) specific to one problematic package. In this case, it's docs examples, which is great, but unnecessary. Unfortunately there was no way to disable or skip that step, so I had to use the remap feature. Again, it worked fine on linux, just not windows because of the backslashes.
Instead of adding package-specific logic to global configs, I think the option to ignore those errors would suffice for most things. Aside from the built-in bin relocation, most of the troublesome packages include docs that aren't 100% necessary. So being able to tell rez to skip that part would be fine for me. Of course this would need to be addressed per package, since the user will need to determine if the extra files are needed or not.
Ah no problem. I just wanted to double down and state that i am generally not a fan :)
This ticket was mentioned in the TSC meeting and I also expressed that pip_install_remap is currently problematic and would need to be revisited.
Also, as a side note, I think that generally speaking, packages that install stuff outside the site-packages directory (excluding the entry points) are kind of badly packaged.
Also, as a side note, I think that generally speaking, packages that install stuff outside the
site-packagesdirectory (excluding the entry points) are kind of badly packaged.
I agree 100%.
+1 as something I'm still coming across. On Windows Rez breaks with issues with escaped characters.
Also, +1 to the concept of seeing what exactly is causing the remapping to be necessary, and for an option to skip it if it's not important.
This will be gone in the new rez-pip.