Skip to content
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

Errors in pip installation part go unnoticed by rez install.py #777

Closed
instinct-vfx opened this issue Oct 28, 2019 · 1 comment · Fixed by #778
Closed

Errors in pip installation part go unnoticed by rez install.py #777

instinct-vfx opened this issue Oct 28, 2019 · 1 comment · Fixed by #778

Comments

@instinct-vfx
Copy link
Contributor

This is with rez-2.47.9 on windows 10 (but this should be cross platform)

The rez install.py script uses subprocess.Popen to call python -m pip install . but ignores returncodes etc. See line 39 ff : https://github.com/nerdvegas/rez/blob/2.47.9/install.py#L39

def run_command(args, cwd=source_path):
    if opts.verbose:
        print("running in %s: %s" % (cwd, " ".join(args)))
    p = subprocess.Popen(args, cwd=source_path)
    p.wait()

If the installation fails this goes unnoticed and still considered a success. A case that i ran into was caused by PYTHONPATH injecting older versions of libraries that caused an error.

I think all i would like to add is checking the return code and raising an exception if the call failed.

Any objections?

@instinct-vfx
Copy link
Contributor Author

Added PR for testing. I only replaced the Popen + p.wait() call with a single check_output() call. In my (windows) testing this worked fine for working installation and raises an exception and aborts installation if the pip call fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants