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

Use os.exec* for pipx run app execution under POSIX. #531

Merged
merged 33 commits into from
Oct 23, 2020

Conversation

itsayellow
Copy link
Contributor

  • I have added an entry to docs/changelog.md

Summary of changes

This changes how a pipx run app is executed.

For POSIX systems (AKA not Windows) it uses os.execvpe() to replace the current pipx process with the app in question. From that point on pipx is not acting as a parent process and is out of the picture.

For Windows, this wasn't a reliable method, so for Windows subprocess.run() and sys.exit() are used to approximate os.exec*.

Because now running an app never returns control to pipx, modifications to deleting a .cache venv when under --no-cache operation had to be made. Previously this was done after running the app. Now, before running the app, a file named pipx_expired_venv is placed inside the venv as a signal to future pipx run invocations to delete this venv during _remove_all_expired_venvs().

Testing was changed to accommodate the new behavior of exiting with an exit code instead of returning via return. The exit code is verified to be correct as needed.

Closes #506

Incidental changes

I renamed util.run() to util.run_verify() to better describe what it is and why it is different from run_subprocess().

Test plan

This should be an internal change to pipx run, and should only be cleaner with respect to certain cases, such as how signals are processed as described in #506 for example. Otherwise pipx run should execute commands with no obvious differences than before.

pipx run pycowsay Mooooooo!

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaborbernat gaborbernat merged commit ac0e8fe into pypa:master Oct 23, 2020
@itsayellow itsayellow deleted the exec branch October 23, 2020 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pipx run isn't redirecting signals to child app (?)
2 participants