-
Notifications
You must be signed in to change notification settings - Fork 340
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.
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
Rez on PyPI #625
Rez on PyPI #625
Conversation
These work better cross-platform than the current hand-made scripts. https://setuptools.readthedocs.io/en/latest/setuptools.html#automatic-script-creation
For working on a fork, like bleeding-rez
Replaced nasty method of fetching PATH from system-wide state, rather than current shell. This doesn't let vcvars.bat do its magic on Windows. More here: AcademySoftwareFoundation@7f45429#r33332549
This currently does not build on Windows, and I can't for the life of me figure out how to fix it. Somewhere, The CMAKE_MODULE_PATH is being populated with a path containing backslash. See error below. CMake Error at C:/Users/manima/AppData/Local/Temp/rez_selftest_o3kxx5/src/packages/sup_world/3.8/build/CMakeFiles/CMakeTmp/CMakeLists.txt:2 (set): Syntax error in cmake code at C:/Users/manima/AppData/Local/Temp/rez_selftest_o3kxx5/src/packages/sup_world/3.8/build/CMakeFiles/CMakeTmp/CMakeLists.txt:2 when parsing string c:/users/manima/dropbox/dev/anima/rez/lib/site-packages/rez-2.29.1-py2.7.egg/rezplugins/build_system/cmake_files;c:\users\manima\appdata\local\temp\rez_selftest_o3kxx5\packages\translate_lib\2.2.0/cmake Invalid character escape '\u'. CMake Error at C:/Program Files/CMake/share/cmake-3.13/Modules/CMakeTestCCompiler.cmake:37 (try_compile): Failed to configure test project build system. -- Configuring incomplete, errors occurred!
# Conflicts: # src/rezplugins/package_repository/filesystem.py
This adds support for hosting Rez in a virtualenv
This reverts commit a164b75. # Conflicts: # src/rezplugins/shell/cmd.py
On Windows, which didn't always find rez, not sure why
Help!I'm having trouble solving the unticked boxes above, primarily the one involving CMake on Windows finding the Visual Studio install.
rez selftest -s cmd
Not searching for unused variables given on the command line.
-- The C compiler identification is unknown
-- The CXX compiler identification is unknown
CMake Error in CMakeLists.txt:
The CMAKE_C_COMPILER:
cl
is not a full path and was not found in the PATH.
To use the NMake generator with Visual C++, cmake must be run from a shell
that can use the compiler cl from the command line. This environment is
unable to invoke the cl compiler. To fix this problem, run cmake from the
Visual Studio Command Prompt (vcvarsall.bat).
Tell CMake where to find the compiler by setting either the environment
variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
the compiler, or to the compiler name if it is in the PATH.
CMake Error in CMakeLists.txt: Any ideas? I suspect I need to add some CMake modules to enable it to find VS on the local disk, but I don't know how. :S |
Having a working pip install would be great. Ditto for automated uploads. I think we should still have an install script to create a virtualenv or standalone python installation. The Also, Rez currently patches the generated console scripts to run with whatever python is in the environment (by default, setuptools bakes in the python executable path when building the script), not sure if we still want to do that or not. |
There could still be, but frankly, I think that's better suited for the README. Installation Choose your installation method. Simple
Recommended
|
src/rezplugins/shell/sh.py
Outdated
@@ -58,6 +59,13 @@ def get_syspaths(cls): | |||
if path not in paths: | |||
paths.append(path) | |||
cls.syspaths = [x for x in paths if x] | |||
|
|||
# add Rez binaries | |||
exe = which("rez") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail under OSX. OSX has already something called rez
, see https://www.unix.com/man-page/osx/1/Rez/. I've never been able to use the rez-commands without -
because of that thing.
[wrk645d:~] jcmorin% which rez
/usr/bin/rez
[wrk645d:~] jcmorin% rez --help
### /Library/Developer/CommandLineTools/usr/bin/Rez - The option --help is not yet implemented.
### /Library/Developer/CommandLineTools/usr/bin/Rez - Usage: /Library/Developer/CommandLineTools/usr/bin/Rez [-a] [-al word | longword] [-arch arch] [-c creator] [-d name[=value]] [-i path] [-isysroot path] [-F framework] [-m] [-noResolve [output | include]] [-o file] [-ov] [-p] [-rd] [-ro] [-s path] [-sc script] [-t type] [-u name] [-useDF] [file...].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JeanChristopheMorinPerso, fixed.
We definitely need to keep the |
I agree that it'd be good to get rid of the embedded dependencies, and that
that should be dealt with separately. I think we can assess this if and
when the new installation is in place. I also think that this should be
done carefully - first step would be removing the most straightforward of
these dependencies, in lieu of its equivalent native package installed into
rez's site (virtualenv or whatever the case may be).
RE an install.py, agreed that this should still exist, even if it becomes
not much more than a utility for performing the relevant virtualenv install
(for eg). I don't want people to waste their time thinking about what the
right install configuration should be for production use - better that this
is codified in the form of an installation script. Bear in mind that rez is
not a project specifically for managing python packages, so it should not
be a requirement to be familiar with pip - it's just as valid for a
C++-orientated developer to want to install it, for eg.
A
…On Thu, Jun 6, 2019 at 12:51 AM JeanChristopheMorinRodeoFX < ***@***.***> wrote:
Yeah, I wasn't implying that it should be included in this PR here. I was
more mentioning that because it will probably affect the installer in some
ways if we review the vendored dependencies. I'll keep watching the
progress of your PR and will see later for creating an issue.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSSUTOXPSRVG7BOTFN3PY7HI5ANCNFSM4HJJFQMA>
.
|
So I've just run a really simple test, and have verified that the issue we hit several years ago is still there. That is, PYTHONPATH is still used in a virtualenv, and takes precedence over any packages installed into the venv itself. It's very easy to break a binary within a virtualenv like this. I used this sample project: https://github.com/pypa/sampleproject. Then:
I installed via wheel because that removes the setuptools dependency in the 'sample' binary (one less thing to worry about). So in any case, this doesn't work. I do have an idea now though - it might be enough to have the rez entry points do this:
Will try this out soon. |
Using those steps, can confirm the same error happens on windows and mac. Another possible idea. You can run the python executable with the |
The installer was also adding I don't know if it's possible to customize the entry point scripts without adding custom code as we do in the installer right now... But here is the doc (https://distlib.readthedocs.io/en/latest/tutorial.html#specifying-scripts-to-install) in case it can help. |
Doh! Of course, should have checked there first. It looks like setuptools supports adding post-install scripts Perhaps just moving the patching code from |
Yes, getting the -E in there is the reason we have patched distlib/virtualenv in the first place. Correct me if I'm wrong but I believe that patching the executables in a post-install routine would be problematic on windows, since these are .exes at that point (rather than a simple shebanged script as per linux). I was thinking about this some more, and I'd like to try out the re-exec idea I mentioned earlier, but only where PYTHONPATH is non-empty. That may do the trick, not sure. On a related note, I recall why we have those embedded dependencies now (vendor/ dir) - or at least, a major reason why. It's because it is a common need to install rez as a rez package - but if rez had external dependencies, then those would also have to be installed as rez packages. To do this would mean either rez itself being able to install packages and their dependencies (which it cannot do yet, for a multitude of reasons, which I want to go over in the roadmap); or, we would have to ensure that rez-pip worked to the extent needed to do this - and that's another complex piece of code that then becomes a potential barrier to installing the rez rez package. Also, at the time, rez-pip did not exist. |
Technically, if I understand things correctly, distlib supports Windows for this. At least there is no warning about Windows in there doc for the shebang. I would guess they do some dark magic (I don't know much about Windows, so everything is dark magic to me 😛 ) ... To be verified I guess.
From what I know, As for the vendored dependencies, I was looking at still having them, but not as we have right now. The archive that we provide (wheel or whatever the format is) would contain the vendored dependencies, but they wouldn't be in git. I'll open an issue so we can discuss future avenues? What do you think? |
JC, yeah I see what you mean RE vendored deps. I do wonder if this should
be left til after any potential changes to installation though, as one may
affect the other.
Thx for the info re exec, will have to look into that. I suppose it
wouldn't have to be an exec - it could just be a subprocess if that avoids
the issues you mention.
A
…On Tue, Jun 11, 2019 at 7:54 AM JeanChristopheMorinRodeoFX < ***@***.***> wrote:
Correct me if I'm wrong but I believe that patching the executables in a
post-install routine would be problematic on windows, since these are .exes
at that point (rather than a simple shebanged script as per linux).
Technically, if I understand things correctly, distlib supports Windows
for this. At least there is no warning about Windows in there doc for the
shebang. I would guess they do some dark magic (I don't know much about
Windows, so everything is dark magic to me 😛 ) ... To be verified I
guess.
I was thinking about this some more, and I'd like to try out the re-exec
idea I mentioned earlier, but only where PYTHONPATH is non-empty. That may
do the trick, not sure.
From what I know, exec caused us a lot of headaches in the past and we've
put some rules where nobody can use them. If I remember correctly,
processes that were spawned with exec where losing their parent or
something, and then if the process was killed, the exec'ed process would
become detached and the process was never really killed. Or something like
that. So for RodeoFX, if we can avoid exec, that would be great. No idea
if you guys had problems in the past with this sort of things.
As for the vendored dependencies, I was looking at still having them, but
not as we have right now. The archive that we provide (wheel or whatever
the format is) would contain the vendored dependencies, but they wouldn't
be in git. I'll open an issue so we can discuss future avenues? What do you
think?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSUMEPOWCYX73SLXM6TPZ3ESTANCNFSM4HJJFQMA>
.
|
So I've done some testing using execve + unsetting PYTHONPATH from environ, and it seems to work so far. JC, I know you said you've had issues with exec, any chance you could find out more? Otherwise I'll go ahead with it anyway and if there are issues we'll find out. At that point it shouldn't be hard to switch to a subprocess. Right now I'm thinking it shouldn't be too hard to put together another PR drawing from this one, but implementing this exec fix and keeping compatibility with the current installer. Then we can put it through some real studio testing and see what breaks. Ps - other evars that can affect rez/python need to be unset also - eg LD_LIBRARY_PATH and LD_PRELOAD. What this won't fix is if someone provides a shared lib on LD_LIBRARY_PATH that overrides one of python's own. I don't know if anyone out there has hit this situation. The libs in question are (Ub 16.04): linux-vdso.so.1 |
Nice find, guys!
I'd be happy to implement this here? You can also push directly to this PR, as I've enabled maintainer push privileges. |
Hey Marcus, no worries if you'd like to tackle it in this PR. To expand a little on what I've been thinking:
|
Cool, having a look at this, and a few more ideas.
Was there any issue with installing |
I feel like we should see how the current approach goes before looking at
alternatives. The shimming that looks to be necessary on Windows could end
up a complicated problem that the current approach may avoid completely.
(1) might work, but the scripts would have to be able to detect if they're
in a venv, and then use the appropriate interpreter. But yeah, linux only.
Thinking about it more and looking back at current installer.py, I think we
still need to perform the step where rez binaries are copied into their own
dir also. There's just too much possibility that a resolved env would
accidentally end up using the venv's python/pip. Consider especially a
resolved env that doesn't contain any python packages. In that case,
running 'python' would definitely use the venv's python, which would be bad
and unexpected (rez shouldn't change anything unrelated to the packages
asked for).
Cheers
A
…On Wed, Jun 12, 2019 at 4:20 PM Marcus Ottosson ***@***.***> wrote:
Cool, having a look at this, and a few more ideas.
1. Revert to using the scripts= argument, and provide .sh and .bat
files, primed with python -E
2. Since (1) isn't nice on Windows (i.e. #626
<#626>), have a look at Shimmy
<https://github.com/pfmoore/shimmy> for making .exe
3. Since (2) is based on Pip's shims which may then also include #616
<#616>, consider
https://github.com/71/scoop-better-shimexe that addresses this problem
Was there any issue with installing .sh scripts on Linux?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSUW5K7P2OWXU5X7D7TP2CISRANCNFSM4HJJFQMA>
.
|
Having a look at this, but not sure I'm doing it right.
Is this what you mean? rez.exe/sh/bat import os
import subprocess
# Clear PYTHONPATH
environ = os.environ.copy()
environ.pop("PYTHONPATH")
# Re-launch Rez with clean env
subprocess.Popen(["rez"], env=environ) If so, then I'm not sure this solves the problem, because in order to get to this point in the Python script, you would first need to find it on the PYTHONPATH/site-packages, which is what the original problem was? E.g. your step (6) and (7) from the sampleproject example. E.g. $ mkdir rez # fake rez
$ touch rez/__init__.py
$ export PYTHONPATH=$(pwd)
$ python -m rez # Wrong Rez picked up And if the wrong Rez is picked up, we couldn't run our code to re-exec a clean Rez. Let me know if I got this wrong somehow. You could also have meant to do this in a |
In essence yes this is the approach, although I'd try an os.execve instead
first. The proc tree in a rez-env is already a bit deep. JC did mention
that they had run into issues exec'ing, but it'd be good to find out for
sure what was involved there, we can always switch to subproc easily enough
if we hit issues.
This approach works if both the rez installation, and the rez rez package,
are in on the game, and both run the same exec'ing code. Of course it is
possible for someone to put a fake rez on PYTHONPATH and break it, but this
is such a narrow case that I'm not concerned by it - if someone wants to go
out of their way to break the system by faking a rez python module, then
whatever! The bigger concern is that current version rez rez package won't
work in combination with this new version. I think we can live with this
caveat because it's a one-off, so long as this incompatibility is made
clear.
…On Thu, Jun 13, 2019 at 6:08 PM Marcus Ottosson ***@***.***> wrote:
Having a look at this, but not sure I'm doing it right.
I'd like to try out the re-exec idea I mentioned earlier, but only where
PYTHONPATH is non-empty. That may do the trick, not sure.
Is this what you mean?
*rez.exe/sh/bat*
import osimport subprocess
# Clear PYTHONPATH
environ = os.environ.copy()
environ.pop("PYTHONPATH")
# Re-launch Rez with clean env
subprocess.Popen(["rez"], env=environ)
If so, then I'm not sure this solves the problem, because in order to get
to this point in the Python script, you would first need to find it on the
PYTHONPATH/site-packages, which is what the original problem was? E.g. your
step (6) and (7) from the sampleproject example.
E.g.
$ mkdir rez # fake rez
$ touch rez/__init__.py
$ export PYTHONPATH=$(pwd)
$ python -m rez # Wrong Rez picked up
And if the wrong Rez is picked up, we couldn't run our code to re-exec a
clean Rez. Let me know if I got this wrong somehow. You could also have
meant to do this in a .sh or .bat script, in which case it might work,
but would eliminate use of entry_points from setup() (which isn't
necessarily a bad thing).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSS226XIIQBHCTDNFHLP2H56BANCNFSM4HJJFQMA>
.
|
Hold up, I thought this was the issue you were demonstrating with sampleproject? Of calling If you're OK with Rez clashing, then perhaps there's an even simpler method that doesn't involve more processes. import sys
sys.path[:] = [] That way, regardless of what was on PYTHONPATH at boot is swept away. We could do this at the start of e.g. While we're at it, we could clean up the environment too. import os
os.environ.pop("PYTHONPATH", None)
os.environ["PATH"] = which("bash") # on Linux
os.environ["PATH"] = which("cmd") # on Windows
You mean as it exists at the moment in this PR? Because this is how I'm using it here and haven't noticed any issues. |
On Fri, Jun 14, 2019 at 4:31 PM Marcus Ottosson ***@***.***> wrote:
Of course it is possible for someone to put a fake rez on PYTHONPATH and
break it
Hold up, I thought this was the issue you were demonstrating with
sampleproject? Of calling sample.exe when another sample/__init__.py was
on the PYTHONPATH?
I just happened to use sample itself to demonstrate the issue. I want to
get to a point where all those embedded vendor deps can just be standard
packages in the rez venv instead, and in that scenario it's more likely
that one of _those_ packages could be interfered with in this way. But that
aside, I know that PYTHONPATH was causing issues in the past that I'm
pretty sure weren't as simple as rez itself being involved (I wish I could
recall one of the cases we hit).
If you're OK with Rez clashing, then perhaps there's an even simpler
method that doesn't involve more processes.
import sys
sys.path[:] = []
That way, regardless of what was on PYTHONPATH at boot is swept away. We
could do this at the start of e.g. rez/cli/_main.py such that any
subsequent import would see nothing but Python's lib/ directory (not even
site-packages/).
I'm not sure that's going to cover other cases such as LD_LIBRARY_PATH on
linux. Even if it works presently, it could cause problems later if and
when rez uses any compiled packages (to put that in context, a C++-based
implementation of the version submodule could speed up the solver
substantially). I also doubt that altering os.environ would do anything for
those cases. We'd have to test to be 100% sure of that though, but library
linking is something that happens outside of the interpreter, I would think
that os.environ affects the interpreter only.
Also, os.execve won't introduce more processes, it will just replace the
current one. If doing this doesn't cause other issues then it'll probably
be the most robust way of removing env vars that can alter rez's behaviour.
While we're at it, we could clean up the environment too.
import os
os.environ.pop("PYTHONPATH", None)
os.environ["PATH"] = which("bash") # on Linux
os.environ["PATH"] = which("cmd") # on Windows
I don't think you'd need to touch PATH, and in fact this is too
simplistic, on linux at least... it's common to have multiple standard
systems paths on PATH, not just the one.
The bigger concern is that current version rez rez package won't work in
combination with this new version.
You mean as it exists at the moment in this PR? Because this is how I'm
using it here and haven't noticed any issues.
Sorry by 'current' I meant any existing rez versions that studios have
installed as a rez package. Both the installed version and the rez rez
package need to have this same bootstrapping code in order for this to work.
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSXFGRXE35BNSNY5BFLP2M3LXANCNFSM4HJJFQMA>
.
|
Ok, I see what you mean. For completeness, these are the unknowns that we're working with at the moment.
For pure-Python packages, editing In either case, better safe than sorry; once we've got a working version covering the unknowns, we can start to peel away at them one at a time. |
Yup, those were my thoughts also.
Cheers
A
…On Fri, Jun 14, 2019 at 5:22 PM Marcus Ottosson ***@***.***> wrote:
Ok, I see what you mean.
For completeness, these are the unknowns that we're working with at the
moment.
- PYTHONPATH was causing issues in the past that I'm pretty sure
weren't as simple as rez itself being involved
- I'm not sure that's going to cover other cases such as
LD_LIBRARY_PATH on linux.
- I also doubt that altering os.environ would do anything for those
cases.
- JC had issues with exec (os.execve or subprocess.call("exec")?)
For pure-Python packages, editing environ and sys.path would have an
effect on imported modules, but I think you're right that os.environ may
not be what compiled modules are actually looking at, and that Python has
an internal os.environ which we aren't able to edit as easily.
In either case, better safe than sorry; once we've have a working version
covering the unknowns, we can start to peel away at them one at a time.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSUSHCDHNPD2DHBKVQ3P2NBKDANCNFSM4HJJFQMA>
.
|
Allow for unsafe environment from within a Rez context
Having a look at this with Instead, I used To test out before/after, I also added $ rez env # pid 1000
> $ # pid 1001
> $ exit # pid 1001 dies
# pid 1000 dies
$ $ export REZ_UNSAFEMODE=1
$ rez env #pid 1000
> $ # still pid 1000 At least two things to improve here.
Despite that, it does work. $ set PYTHONPATH=some/bad/path
$ rez env
> $ echo $PATH
> $ |
RE execve and REZ_UNSAFEMODE: I fully expect there may be differences between platforms. For example, Linux may be fine with execve, and if so we should use it. I also think we may find ourselves in need of multiple settings that relate to this bootstrap-level stuff. I have a suggestion: we should have one _REZ_BOOTSTRAP_CONFIG evar, which is a JSON-encoded dict. Also, for each platform, we have a hardcoded dict which contains the defaults for these settings. In _main, this dict is updated with anything from _REZ_BOOTSTRAP_CONFIG. So for eg:
This will avoid a bunch of extra evars if we find we need further control; it also gives us a nice analogy to the main config (which we can't use in this case, of course). It's a little technical to have to provide JSON, but in this case I think it's good - anyone changing this stuff should be fairly technical. In terms of whether or not you can avoid the extra subproc on Windows, I have no idea :) """Is there another way of getting the new Python to launch rez""" I would have thought you'd just be using the exact same sys.argv again? Presumably this would then just invoke the very same entry point again. Is there a reason you aren't doing this? |
Oh, I would have thought you'd prefer consistency, despite platform. If subprocess works on Linux too, then why implement it twice? They're bound to have issues of their own. Are you sure?
That's a good idea, once related variables start growing that's one way of handling it.
It does forward the arguments, but it needs to take into account the calling executable which isn't Python itself, but rather the compiled executable. $ rez env # print(sys.argv)
['C:\\Users\\marcus\\rez-virtualenv\\Scripts\\_rez\\rez.exe', 'env'] In the current commit, it's replacing |
On Tue, Jun 18, 2019 at 10:47 PM Marcus Ottosson ***@***.***> wrote:
For example, Linux may be fine with execve, and if so we should use it
Oh, I would have thought you'd prefer consistency, despite platform. If
subprocess works on Linux too, then why implement it twice? They're bound
to have issues of their own. Are you sure?
If we have to use an extra subprocess on Windows then that's just what we
have to live with. I suspect that on linux, using execve instead will work
and will avoid the extra subprocess, and I think this is worth the cost in
consistency. At the end of the day though, what matters is that the
behaviour is as expected. Bear in mind that this only affects invocation of
the rez cli tools, certainly if the effect was more widespread I would have
more reservations about differing implementation across compilers.
I have a suggestion: we should have one _REZ_BOOTSTRAP_CONFIG evar
That's a good idea, once related variables start growing that's one way of
handling it.
I would have thought you'd just be using the exact same sys.argv again?
Presumably this would then just invoke the very same entry point again. Is
there a reason you aren't doing this?
It does forward the arguments, but it needs to take into account the
calling executable which isn't Python itself, but rather the compiled
executable.
$ rez env # print(sys.argv)
['C:\\Users\\marcus\\rez-virtualenv\\Scripts\\_rez\\rez.exe', 'env']
In the current commit, it's replacing rez.exe with python -m rez and
forwards the rest.
I understand that that's what you're doing, I just am not clear as to why.
If you clear PYTHONPATH and then start a subproc with the exact same argv,
does that not work? Perhaps I'm missing something.
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#625>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMOUSWBZOGGB4KEI44GOD3P3DKOPANCNFSM4HJJFQMA>
.
|
Now the setup.py is able to make binary scripts by itself and install them as data_files under rez production dir 'bin/rez' (or 'Scripts/rez' on Windows). However, console_scripts 'rez' is conflicting with the rez production dir 'rez'. Since the previous install.py already removing those scripts before doing the *patch*, console_scripts should be ignored in new setup.py. See AcademySoftwareFoundation#625 and AcademySoftwareFoundation#662 for reason why adding console_scripts in the first place.
Closing as we're already publishing to pypi via gh workflow now |
Hi all,
Here's a version of Rez compatible with PyPI.
pip install bleeding-rez
Try latest branch
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
Ticked boxes mean I'm happy with it, but let me know if you aren't. Unticked means it's something we need to address.
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: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, you can either can install any PyPI package in a virtualenv, including this Rez from this PR.
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 replaced with using 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
CODECOV_TOKEN
to AppVeyor's EnvironmentLet me know your thoughts!
Edit: This also closes #339