rez icon indicating copy to clipboard operation
rez copied to clipboard

Add --favor-paths rez-env flag

Open ColinKennedy opened this issue 3 years ago • 2 comments

Closes: https://github.com/nerdvegas/rez/issues/1263

Added rez-env cli option, --favor-paths

  • If not provided, does nothing
  • If provided and 1+ path specified, --favor-paths /foo:/bar
    • All packages found within /foo are preferred over packages found in /bar
    • All packages found in /bar are preferred over packages found within packages_path
  • If provided but no paths specified, --favor-paths, [config.local_packages_path] is used. All other logic applies.
  • In all cases, the usual package path rules still apply. e.g. any path provided in --favor-paths must be in packages_path or it won't be used during ordering

Miscellaneous

Introduced SplitPaths to show an alternative way we can be parsing our arguments. If you like that approach, I can add it to --paths as well, which is currently being described in setup_parser but then parsed again, in env.py command.

Question To Reviewers

I was thinking of also including a new rez.config.Config option for favor paths, so that people can set it via environment variable as well. I can think of some situations where this would be really desirable. Would that be okay to add?

What's not in this PR, currently

And now I recall - the problem we ran into when discussing this ages ago, is that orderers can't be stacked. So that could interfere with orderers already in place.

In the original ticket thread, https://github.com/nerdvegas/rez/issues/1263#issuecomment-1086347517, @nerdvegas mentions that we may need a kind of "stacked orderer". However I didn't encounter any limitations in practice. ResolvedContext takes a list of package orderers so it should be fine. However please let me know if not and I'll make necessary adjustments.

ColinKennedy avatar May 07 '22 21:05 ColinKennedy

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar May 07 '22 22:05 sonarqubecloud[bot]

Hi @ColinKennedy , thanks for this PR, super useful!

We have been testing it in Windows and found one issue that I think should be easy to resolve.

It looks like Rez implements a canonical_path function, that amongst other things, converts the package locations to lowercase for case insensitive filesystems: https://github.com/AcademySoftwareFoundation/rez/blob/71f2f612223dc7c1c88ea146dd0d06ac1ea45ad0/src/rezplugins/package_repository/filesystem.py#L500

canonical_path definition: https://github.com/AcademySoftwareFoundation/rez/blob/71f2f612223dc7c1c88ea146dd0d06ac1ea45ad0/src/rez/utils/filesystem.py#L528

In FavorPathsOrder you are checking if the package location is in the list of favor paths: https://github.com/AcademySoftwareFoundation/rez/pull/1305/files#diff-772d1d5249a49e66b7497839f3f80ddb367c98ee0c880a8a3b2edf421ae817feR174

this comparison should be done with canonical_path in mind otherwise the check most likely always fail in Windows. ie. windows drive letters tend to be uppercase.

To solve it there are a couple of obvious options,

  • either in the __init__ of the package ordered,
from rez.utils.filesystem import canonical_path

...

self._paths = [canonical_path(path) for path in paths]

or later on in the compoarison:

from rez.utils.filesystem import canonical_path

...
    def reorder(self, iterable, key=None):
...
        buckets = collections.OrderedDict([(canonical_path(path), []) for path in self._paths])

Hope this helps, I've tested both with our Windows setup and they work as expected.

diegogarciahuerta avatar Jan 09 '23 06:01 diegogarciahuerta