Installer updates #662

nerdvegas · 2019-07-12T07:26:20Z

This PR updates the current installer. Changes include:

patched distlib (in build_utils) has been removed. The patch we were relying on has since been made part of the main distlib release, which we already have vendored;
virtualenv has been updated to latest;
scripts have been removed, and entry points are used instead;
installation occurs via wheel;
install.py code has been cleaned up and simplified. Specifically, standard use of distlib.ScriptMaker has been put in place;
INSTALL.md has been updated with a full explanation of the installer, and why a pip-based installation is not the same as using install.py. Reviewers are encouraged to read this file.

It would be great if others could test this, especially Windows users.

-using vendor.distlib instead - new version now incorporates patch previously relied on -see https://bitbucket.org/pypa/distlib/commits/bff28df

-added _entry_points module -misc fixes in setup.py

-removed hacky ScriptMaker code in install.py

nerdvegas · 2019-07-12T07:32:33Z

Note also that this PR supersedes #625, although I'm interested in incorporating parts of that into separate PRs (notably, automated pip uploads). I'm also interested in Appveyor, which also belongs in a separate PR (and there is one - #339 - which should be compared with what's available from #625 and potentially combined).

instinct-vfx · 2019-07-12T08:00:06Z

Will test this today.

instinct-vfx · 2019-07-12T09:10:31Z

I just gave this a quick run to test the pip installation. The behavior does not match INSTALL.md.

However, this comes with a caveat - rez command line tools are disabled once inside a rez environment (ie after using the rez-env command). The reasons are given in the next section.

I did the following to install rez through pip:

python setup.py bdist_wheel
pip install rez-2.36.0-py2-none-any.whl

Now Rez executables still work inside resolved shells though:

This should not work, should it?

btw. please let me know if you prefer this to be comments in the diff section i can also move it there.

instinct-vfx · 2019-07-12T10:18:09Z

Also there seems to be an issue with paths containing ~. As our deployment setup installs from a tempfolder generated location this can contain ~ if the corresponding user profile has certain properties (like mine does). Still investigating what change exactly causes that issue and if it is something that should be fixed in Rez or in our setup.

mottosso

Great stuff! I've added my two cents, from my humble experience with Rez on PyPI.

mottosso · 2019-07-12T20:10:03Z

INSTALL.md

+]$ pip install rez
+```
+
+However, this comes with a caveat - _rez command line tools are disabled once


One solution may be to append the parent directory of which("rez") in the get_syspaths() method, here. This would also work on Windows.

mottosso · 2019-07-12T20:11:07Z

INSTALL.md

+Specifically:
+
+* When within a rez environment (ie after using the `rez-env` command), the rez
+  command line tools would not be guaranteed to function correctly;


If the above method works, then this may be checked off safely.

mottosso · 2019-07-12T21:25:07Z

INSTALL.md

+* When within a rez environment (ie after using the `rez-env` command), the rez
+  command line tools would not be guaranteed to function correctly;
+* When within a rez environment, other packages' tools (that were also installed
+  with pip) would remain visible, but would not be guaranteed to work.


This may be addressed using one of the methods you mentioned in #625.

I made a prototype of this a few weeks ago but haven't been testing it in production for more than a few days, and only on Windows. I've updated it to work on Linux just now, you can test it and see if it fits the bill like this:

$ pip install bleeding-rez $ rez --version bleeding-rez-2.33.4 $ rez env python --isolated

The environment you'll get is completely void of anything from the parent environment, including any PYTHONPATH or site-packages. Now, this is so barebones, that it also excludes rez itself from PATH. So if you wanted it there, you could add it here, where bash and lsb_release are explicitly added as well.

Alternatively, the REZ_ENVIRONMENT variable takes a JSON-serialised environment to be used in place of the skeleton environment built into the source. Out of the box you're left with a Docker-like environment, with only the shell itself plus your resolve.

mottosso · 2019-07-12T21:25:43Z

INSTALL.md

+When you enter a rez environment, the rez packages in the resolve configure
+that environment as they see fit. For example, it is not uncommon for a python
+package to append to PYTHONPATH. Environment variables such as PYTHONPATH
+affect the behaviour of tools, including rez itself, and this can cause it to


This would possibly be addressed by the above as well.

mottosso · 2019-07-12T21:26:36Z

INSTALL.md

+to avoid this problem. Specifically:
+
+* Rez is installed into a virtualenv so that it operates standalone;
+* The rez tools are shebanged with `python -E`, in order to protect them from


If the --isolated method works, the python -E is embedded into Rez, which means we could skip this particular step.

mottosso · 2019-07-12T21:27:04Z

INSTALL.md

+* Rez is installed into a virtualenv so that it operates standalone;
+* The rez tools are shebanged with `python -E`, in order to protect them from
+  environment variables that affect python's behaviour;
+* The rez tools are stored in their own directory, so that other unrelated tools


And finally, due to explicit control over the environment, this would also go away. Pof!

nerdvegas · 2019-07-12T21:47:59Z

Are you referring to the subprocess approach? I considered this, however the install.py-based installation works without the need for an extra subproc. With that in mind, it doesn't make sense to support two different installation methods, when one results in differing behaviour like this. However, if enough people feel a great need for production-ready pip installable rez, we could take that approach, _if_ it's isolated to pip installations only (ie, install.py should continue doing what it currently does). If contributed, this belongs in a separate PR. A

…

On Sat, Jul 13, 2019 at 7:31 AM Marcus Ottosson ***@***.***> wrote: ***@***.**** commented on this pull request. Great stuff! I've added my two cents, from my humble experience with Rez on PyPI. ------------------------------ In INSTALL.md <#662 (comment)>: > +``` + +Do _not_ move the installation - re-install to a new location if you want to +change the install path. If you want to install rez for multiple operating +systems, perform separate installs for each of those systems. + + +# Installation Via Pip + +It is possible to install rez with pip, like so: + +``` +]$ pip install rez +``` + +However, this comes with a caveat - _rez command line tools are disabled once One solution may be to append the parent directory of which("rez") in the get_syspaths() method, here <https://github.com/nerdvegas/rez/blob/installer-updates/src/rezplugins/shell/sh.py#L36>. This would also work on Windows. ------------------------------ In INSTALL.md <#662 (comment)>: > +inside a rez environment (ie after using the `rez-env` command)_. The reasons +are given in the next section. + +Rez installation via pip is not considered production ready. However, if all you +want is the rez API, and you don't need its tools to be available within rez +environments, then you can install with pip. + + +# Why Not Pip For Production? + +Rez is not a normal python package. Although it can successfully be installed +using standard mechanisms such as pip, this comes with a number of caveats. +Specifically: + +* When within a rez environment (ie after using the `rez-env` command), the rez + command line tools would not be guaranteed to function correctly; If the above method works, then this may be checked off safely. ------------------------------ In INSTALL.md <#662 (comment)>: > + +Rez installation via pip is not considered production ready. However, if all you +want is the rez API, and you don't need its tools to be available within rez +environments, then you can install with pip. + + +# Why Not Pip For Production? + +Rez is not a normal python package. Although it can successfully be installed +using standard mechanisms such as pip, this comes with a number of caveats. +Specifically: + +* When within a rez environment (ie after using the `rez-env` command), the rez + command line tools would not be guaranteed to function correctly; +* When within a rez environment, other packages' tools (that were also installed + with pip) would remain visible, but would not be guaranteed to work. This may be addressed using one of the methods you mentioned in #625 <#625>. I made a prototype of this a few weeks ago but haven't been testing it in production for more than a few days, and only on Windows. I've updated it to work on Linux just now, you can test it and see if it fits the bill like this: $ pip install bleeding-rez $ rez --version bleeding-rez-2.33.4 $ rez env python --isolated The environment you'll get is completely void of anything from the parent environment, including any PYTHONPATH or site-packages. Now, this is so barebones, that it also excludes rez itself from PATH. So if you wanted it there, you could add it here <https://github.com/mottosso/bleeding-rez/blob/d99c7baf65dd4b82ab72e80961c0d77aaefa36be/src/rez/cli/_main.py#L152-L157>, where bash and lsb_release are explicitly added as well. Alternatively, the REZ_ENVIRONMENT variable takes a JSON-serialised environment to be used in place of the skeleton environment built into the source. Out of the box you're left with a Docker-like environment, with only the shell itself plus your resolve. ------------------------------ In INSTALL.md <#662 (comment)>: > + +# Why Not Pip For Production? + +Rez is not a normal python package. Although it can successfully be installed +using standard mechanisms such as pip, this comes with a number of caveats. +Specifically: + +* When within a rez environment (ie after using the `rez-env` command), the rez + command line tools would not be guaranteed to function correctly; +* When within a rez environment, other packages' tools (that were also installed + with pip) would remain visible, but would not be guaranteed to work. + +When you enter a rez environment, the rez packages in the resolve configure +that environment as they see fit. For example, it is not uncommon for a python +package to append to PYTHONPATH. Environment variables such as PYTHONPATH +affect the behaviour of tools, including rez itself, and this can cause it to This would possible be addressed by the above as well. ------------------------------ In INSTALL.md <#662 (comment)>: > +* When within a rez environment (ie after using the `rez-env` command), the rez + command line tools would not be guaranteed to function correctly; +* When within a rez environment, other packages' tools (that were also installed + with pip) would remain visible, but would not be guaranteed to work. + +When you enter a rez environment, the rez packages in the resolve configure +that environment as they see fit. For example, it is not uncommon for a python +package to append to PYTHONPATH. Environment variables such as PYTHONPATH +affect the behaviour of tools, including rez itself, and this can cause it to +crash or behave abnormally. + +When you use the `install.py` script to install rez, some extra steps are taken +to avoid this problem. Specifically: + +* Rez is installed into a virtualenv so that it operates standalone; +* The rez tools are shebanged with `python -E`, in order to protect them from If the --isolated method works, the python -E is embedded into Rez, which means we could skip this particular step. ------------------------------ In INSTALL.md <#662 (comment)>: > +* When within a rez environment, other packages' tools (that were also installed + with pip) would remain visible, but would not be guaranteed to work. + +When you enter a rez environment, the rez packages in the resolve configure +that environment as they see fit. For example, it is not uncommon for a python +package to append to PYTHONPATH. Environment variables such as PYTHONPATH +affect the behaviour of tools, including rez itself, and this can cause it to +crash or behave abnormally. + +When you use the `install.py` script to install rez, some extra steps are taken +to avoid this problem. Specifically: + +* Rez is installed into a virtualenv so that it operates standalone; +* The rez tools are shebanged with `python -E`, in order to protect them from + environment variables that affect python's behaviour; +* The rez tools are stored in their own directory, so that other unrelated tools And finally, due to explicit control over the environment, this would also go away. Pof! — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#662>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUSQD7UKCVYYSAZ2DEOLP7DZ45ANCNFSM4ICDSWPA> .

nerdvegas · 2019-07-12T21:52:49Z

Ah, it may be that the tool disabling only works in a virtualenv-based pip installation. Come to think of it, yeah there's no reliable way to remove visibility of rez tools without taking out a bunch of other stuff as well. I'll update the docs in this PR.

…

On Fri, Jul 12, 2019 at 7:10 PM Thorsten Kaufmann ***@***.***> wrote: I just gave this a quick run to test the pip installation. The behavior does not match INSTALL.md. However, this comes with a caveat - rez command line tools are disabled once inside a rez environment (ie after using the rez-env command). The reasons are given in the next section. I did the following to install rez through pip: python setup.py bdist_wheel pip install rez-2.36.0-py2-none-any.whl Now Rez executables still work inside resolved shells though: [image: image] <https://user-images.githubusercontent.com/243828/61116860-921d8000-a495-11e9-95ba-176b232f70b7.png> This should not work, should it? btw. please let me know if you prefer this to be comments in the diff section i can also move it there. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#662>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUSSPFL25J6MKR3GBZTLP7BDATANCNFSM4ICDSWPA> .

JeanChristopheMorinPerso · 2019-07-13T15:42:36Z

install.py

+    _, pip_executable = get_venv_executable(dest_dir, "pip")
+
+    # build wheel
+    run_command([py_executable, "setup.py", "bdist_wheel"])


I think we can simplify and solidify this by doing:

run_command([py_executable, "-m", "pip", "install", "."])

Using py_executable -m pip you don't need to take care of finding pip. Python will do it for us.

pip install . will build the wheel for you and install it for you. It is a little cleaner and a preferred approach from what I know. The other benefit is that you don't have to look for the wheel in the dist directory which could have some development stuff in there is not cleaned-up before running the install.py file.

nice, changes coming

JeanChristopheMorinPerso · 2019-07-13T16:02:20Z

setup.py

@@ -13,11 +13,20 @@
    sys.exit(1)


-if sys.version_info < (2, 6):
-    print("install failed - requires python v2.6 or greater", file=sys.stderr)
+if sys.version_info < (2, 7):


While we are there, we should also check for python 3. And we should also set the python_requires argument in the setup function.

A py3 port is coming soon (another dev is working on it) so I think I'll leave this for now.

JeanChristopheMorinPerso · 2019-07-13T17:01:47Z

src/rez/cli/_entry_points.py

+@scriptname("rez-build")
+def run_rez_bind():
+    from rez.cli._main import run
+    return run("bind")


I think you copy/pasted a little too fast 😛 It should be build and the function name should also contain build as right now it override the bind. Same for rez-config.

thanks for the catch, fixes coming

JeanChristopheMorinPerso · 2019-07-13T18:16:57Z

Ah, it may be that the tool disabling only works in a virtualenv-based pip
installation.

Come to think of it, yeah there's no reliable way to remove visibility of
rez tools without taking out a bunch of other stuff as well. I'll update
the docs in this PR.

Yep, tool disabling only works when installing with install.py. We could potentially detect from the rez tools if we are not in recommended deployment method and log a warning maybe? I know some tools that does this (pipenv is the first to come to my mind). Or simply block the usage. We could call rez-system.is_production_rez_install from https://github.com/nerdvegas/rez/blob/master/src/rez/cli/_main.py#L72 and spit out a warning if it's false: We detected that Rez was installed with Pip. Please be aware the rez command line tools are not guaranteed to function correctly. See <link to wiki> for more information.

What do you think?

JeanChristopheMorinPerso · 2019-07-13T18:24:26Z

INSTALL.md

@@ -1,3 +1,85 @@
 # Installation

 See https://github.com/nerdvegas/rez/wiki/Getting-Started#installation
+
+
+First, install Rez. Download the source, and, from the source directory, run:


Philosophical question (sorry in advance): Should we move this doc in the https://github.com/nerdvegas/rez/wiki/Getting-Started#installation doc? I'm also wondering if we should add a mention about how to install in the README as it's usually a common practice to do so.

Or we want to tackle that into a separate subject and wait for your roadmap before deciding?

Yes to both, will make those changes.

instinct-vfx · 2019-07-15T09:29:31Z

Also there seems to be an issue with paths containing ~. As our deployment setup installs from a tempfolder generated location this can contain ~ if the corresponding user profile has certain properties (like mine does). Still investigating what change exactly causes that issue and if it is something that should be fixed in Rez or in our setup.

Please ignore this one, i found the issue and it is not within Rez. Sorry for that.

-warning msg on cli tools added for non-pip installation -simplified setup.py

JeanChristopheMorinPerso · 2019-07-16T02:20:47Z

src/rez/cli/_entry_points.py

+
+@scriptname("rez")
+def run_rez():
+    check_production_install()


I'm wondering if you had a reason to not move that into the rez.cli._main.run. I have no problem if we do the check here. The only reason why I was mentioning rez.cli._main is because that would allow us to remove some duplication and have the check more centralized. Maybe you see a future where one command would not need the check?

nerdvegas · 2019-07-16T03:22:41Z

The reason really is just to get this code into the callstack as early as possible. The more rez code we import in a non-production-safe install, the more chance something goes wrong before the warning is printed. A

…

On Tue, Jul 16, 2019 at 12:20 PM Jean-Christophe Morin < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/rez/cli/_entry_points.py <#662 (comment)>: > + filepath = os.path.join(path, ".rez_production_install") + + if not os.path.exists(filepath): + sys.stderr.write( + "Pip-based rez installation detected. Please be aware that rez command " + "line tools are not guaranteed to function correctly in this case. See " + "https://github.com/nerdvegas/rez/wiki/Installation#why-not-pip-for-production " + " for futher details.\n" + ) + + +### Entry points + ***@***.***("rez") +def run_rez(): + check_production_install() I'm wondering if you had a reason to not move that into the rez.cli._main.run. I have no problem if we do the check here. The only reason why I was mentioning rez.cli._main is because that would allow us to remove some duplication and have the check more centralized. Maybe you see a future where one command would not need the check? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#662>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUSRNEU6IHKTL3ZUHZ2LP7UWAFANCNFSM4ICDSWPA> .

JeanChristopheMorinRodeoFX · 2019-07-17T00:09:51Z

It makes a lot of sense.

I'm also wondering if we should write some tests to make sure it behaves as we want and that it stays like this in the future. With potentially all the cases we found in the last couple of weeks.

What do you think? Not too sure how we would go on testing that though.

nerdvegas · 2019-07-17T00:16:33Z

Yeah perhaps, but let's leave it on ice for the moment. There's a lot to do, we'll see if any problems arise. A

…

On Wed, Jul 17, 2019 at 10:09 AM JeanChristopheMorinRodeoFX < ***@***.***> wrote: It makes a lot of sense. I'm also wondering if we should write some tests to make sure it behaves as we want and that it stays like this in the future. With potentially all the cases we found in the last couple of weeks. What do you think? Not too sure how we would go on testing that though. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#662>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUSTD7AMZN662QJ6IZU3P7ZPNDANCNFSM4ICDSWPA> .

nerdvegas · 2019-07-19T09:54:08Z

Can anyone confirm that this has been tested on Windows? I think we're ready to merge if so.

instinct-vfx · 2019-07-19T10:05:33Z

I tried it and it broke for me in various places. I am not sure if this is our setup though. Will do a vanilla install and report later today. Sorry for the delay. Too many things going on at once he.

instinct-vfx

After setting things up properly and deploying using our deployment setup everything seems to be working fine as far as i can tell. Sorry again for the delays.

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.

ajohns added 7 commits July 12, 2019 08:51

-removed patched distlib in build_utils

b7bd358

-using vendor.distlib instead - new version now incorporates patch previously relied on -see https://bitbucket.org/pypa/distlib/commits/bff28df

-updated to latest virtualenv

a3d53ea

-removed tmp files

435b4be

-WIP

81517a9

-added _entry_points module -misc fixes in setup.py

-switched to entry points

8bb396d

-removed hacky ScriptMaker code in install.py

-removed bin dir

16532fe

update INSTALL docs

b8b08b5

nerdvegas added the enhancement label Jul 12, 2019

nerdvegas requested review from pmolodo, instinct-vfx and maxnbk July 12, 2019 07:26

nerdvegas self-assigned this Jul 12, 2019

mottosso reviewed Jul 12, 2019

View reviewed changes

doc update

3172b08

JeanChristopheMorinPerso reviewed Jul 13, 2019

View reviewed changes

-install docs updated and copied into wiki

b866c29

-warning msg on cli tools added for non-pip installation -simplified setup.py

JeanChristopheMorinPerso reviewed Jul 16, 2019

View reviewed changes

instinct-vfx approved these changes Jul 19, 2019

View reviewed changes

nerdvegas merged commit b866c29 into master Jul 20, 2019

lambdaclan mentioned this pull request Jul 29, 2019

[WIP-FIX] Remove the shell=True globally from Windows Python commands (pip bind, version check) #681

Closed

1 task

davidlatwe mentioned this pull request Mar 12, 2021

Install as Rez bin tool davidlatwe/rez-deliver#6

Closed

3 tasks

bpabel deleted the installer-updates branch January 19, 2023 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Installer updates #662

Installer updates #662

nerdvegas commented Jul 12, 2019 •

edited

Loading

nerdvegas commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

mottosso left a comment

mottosso Jul 12, 2019

mottosso Jul 12, 2019

mottosso Jul 12, 2019

mottosso Jul 12, 2019 •

edited

Loading

mottosso Jul 12, 2019

mottosso Jul 12, 2019

nerdvegas commented Jul 12, 2019 via email

nerdvegas commented Jul 12, 2019 via email

JeanChristopheMorinPerso Jul 13, 2019

nerdvegas Jul 15, 2019

JeanChristopheMorinPerso Jul 13, 2019

nerdvegas Jul 15, 2019

JeanChristopheMorinPerso Jul 13, 2019 •

edited

Loading

nerdvegas Jul 15, 2019

JeanChristopheMorinPerso commented Jul 13, 2019

JeanChristopheMorinPerso Jul 13, 2019

nerdvegas Jul 15, 2019

instinct-vfx commented Jul 15, 2019

JeanChristopheMorinPerso Jul 16, 2019

nerdvegas commented Jul 16, 2019 via email

JeanChristopheMorinRodeoFX commented Jul 17, 2019

nerdvegas commented Jul 17, 2019 via email

nerdvegas commented Jul 19, 2019

instinct-vfx commented Jul 19, 2019

instinct-vfx left a comment

Installer updates #662

Installer updates #662

Conversation

nerdvegas commented Jul 12, 2019 • edited Loading

nerdvegas commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

instinct-vfx commented Jul 12, 2019

mottosso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mottosso Jul 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerdvegas commented Jul 12, 2019 via email

nerdvegas commented Jul 12, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeanChristopheMorinPerso Jul 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeanChristopheMorinPerso commented Jul 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

instinct-vfx commented Jul 15, 2019

Choose a reason for hiding this comment

nerdvegas commented Jul 16, 2019 via email

JeanChristopheMorinRodeoFX commented Jul 17, 2019

nerdvegas commented Jul 17, 2019 via email

nerdvegas commented Jul 19, 2019

instinct-vfx commented Jul 19, 2019

instinct-vfx left a comment

Choose a reason for hiding this comment

nerdvegas commented Jul 12, 2019 •

edited

Loading

mottosso Jul 12, 2019 •

edited

Loading

JeanChristopheMorinPerso Jul 13, 2019 •

edited

Loading