Closed mottosso closed 5 years ago
Stage 1 may bring up an issue for some depending on how Rez is provided to end-users. For us this won't be an issue i think but for reference and discussion i would like to add some information.
AFAIK (and that is not that far, so there may be other reasons?) the main reason for patching the executables in the installer is to have a single directory with ONLY the Rez commands. That way you can safely add this folder to %PATH% to provide Rez commands to end-users.
If the commands reside in Scripts\ (or bin\ on linux) adding that folder to %PATH% also adds all other scripts of the specific python version to %PATH%. With multiple python versions installed this may be a problem.
I am pretty sure fetching PATH from the current session creates unexpected problems. I have been bitten by the implementation before, but i think that weakening the separation between Rez's own execution environment and the environments it creates is not a good idea. I'd like to hear Allan's feedback here. I think i would prefer to just enable cmake usage without using vcvarsall. If you have to manually select a version that could also happen inside a resolved shell, no?
Thanks @instinct-vfx just FYI this PR isn't actually on the Rez repo yet, so we'll possibly have to re-have our conversation later. I'm keeping this here to format my WIP markdown message before submitting.
the main reason for patching the executables in the installer is to have a single directory with ONLY the Rez commands.
Yes, this was my impression too. At the moment, you'd get Rez, but also anything else you already had in your Scripts/
directory which isn't very clean.
In this case, I think one could still safely use their own virtualenv (i.e. no need to embed/force one).
$ python -m virtualenv my_safe_rez
(my_safe_rez) $ pip install rez
Whereby you could then also copy/pasta the virtualenv directory wherever, like you can with it embedded.
I am pretty sure fetching PATH from the current session creates unexpected problems.
Yes, I expect to discover those problems first-hand; at the moment, it solved at least one issue and at surface level appears a step in the right direction.
Production-wise, it would be identical to fetching from the system PATH in all cases but one; when the user has (1) entered a shell, (2) made some changes, and (3) then entered Rez. It's the (2) that's the killer, but that's highly manageable I would imagine? I.e. on automated tasks to a render farm, a shell would be spawned (inheriting system PATH) and Rez immediately executed. I.e. on an app launcher script, the app launcher itself would control whether (or not) to modify PATH prior to calling Rez. So I really don't see the problem here, other than being careful just because we can.
I think that from a Rez point of view the problem is not really (2) as that's the whole point (the user making changes to control behavior). The problem is, that rez commands are also available from within Rez created shells. And at that point things can get really ugly i assume (and that's really just an assumption, i'd like to hear Allan's thoughts!)
Having seen this particular issue of PATH around the mailing list and several GitHub issues over the past few days, I've changed my mind. It appears well thought through, and there's no reason not to keep it, other than fixing the tests that break with out it.
NOTE: WIP PR
Not ready to submit just yet
Hi all,
Here's a version of Rez compatible with PyPI, that as a result is somewhat slimmer.
Try current version
All tests pass on both Windows and Linux (the PR includes an AppVeyor Windows environment for tests). See below for current minor obstacles.
Overview
I've approached this in stages, whereby you are welcome to merge either one or all.
scripts/
directory to ["entry points"](), so as to get rid of the manual creation of binaries on all platformsinstall.pyis no more I got rid ofinstall.py
and all that goes with it (i.e.build_utils/
)Highlights
Let me know if any of my assertions are wrong; they are based on looking through the Git history comments but some amount of filler has been necessary!
bleeding-rez
, which is what you're seeing above. There's not much to say about it other than it works as you would expect. No tricks.cmake
, and thuscmake
tests were never run. I did however run into the following issue..vcvarsall.bat
doesn't work with the way thecmd
shell is implemented;cmd
is inheritingPATH
from the system registry, whereas Visual Studio assumesPATH
is appended interactively. As a consequence of this, it does mean Rez now inheritsPATH
more generally, which may or may not be what we want. See 1system.is_production_installis no more This problem may be better addressed in another way, see 2CMAKE_MODULE_PATH
got appended with a Windows-backslash which threw cmake. See commit message for details.--target
Custom install directory with--target
doesn't work with how Rez manages yourPYTHONPATH
. Because if you do use--target
you would also need to manually add Rez to your PYTHONPATH but you can't, sincerez env
will take control of that PYTHONPATH and remove it once you've entered into an environment. Without--target
, rez will still be accessible assite-packages
is accessible despite not being on yourPYTHONPATH
. Workaround is adding the target to your./~bashrc
on Linux and system PATH on Windows.Standing Issues
No Virtual Envrionment
From what I can tell, the embedded virtualenv was used to let packages gain absolute control over
PYTHONPATH
, whilst still being able to expose therez
Python package torez env
.Now that there is no virtual environment,
rez
is made available via thesite-packages
of the target Python install.But that means you can't install Rez elsewhere anymore, which can be a problem if you host Rez itself as a package.
To solve that, I can think of two options.
sys.path
.(1) is simple enough, as you can install any PyPI package in a virtualenv, including this Rez from this PR.
(2) is a feature addition, as far as I can tell, via e.g. adding a site package path, and inserting to
sys.path.insert(0,
os.path.dirnam(rez)` on entering an environment.So the question is, can the original problem be resolved (pun!) by either (1) or (2)?
PATH
On Linux, the environment inherited by Rez is coming the call
sh echo $PATH
which reads its PATH from the usual suspects.On Windows, prior to this PR, PATH was coming from the Windows registry; in similar vain to the above Bash scripts.
In this PR however, it's coming from
os.environ["PATH"]
for the sole purpose of enabling tests involving CMake to pass on Windows.Is Production Install
Previously, a file called
.is_production_install
was embedded in theScripts\rez
directory alongside the Rez binaries. I was placed there to help identify the install having been made viainstall.py
, which in turn was designed to solve the aforementioned issue ofPATH
.With this PR, that safety guard is off per default, and made possible by installing into your own virtual environment.
The advantage is two-fold.
I'd imagine the README would then be split between a "I just want to take Rez for a spin" and "I like Rez, now I want to use it in my large corp", whereby the latter involves a virtual env.
Post-merge
In order to test deployment, I've deployed to my account under a different name and version.
Once merged, you'll need to:
appveyor.yml