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

Update hooks for pyqtgraph #501

Merged
merged 4 commits into from
Oct 20, 2022
Merged

Update hooks for pyqtgraph #501

merged 4 commits into from
Oct 20, 2022

Conversation

rokm
Copy link
Member

@rokm rokm commented Oct 19, 2022

Update pyqtgraph standard hook to collect all data from the package, including the colormaps and their associated license files, as per #490 (comment).

Create a new runtime hook that attempts to handle subprocess spawned by pyqtgraph.multiprocess. The caveat is that the user still needs to call stdlib multiprocessing.freeze_support in their entry-point script (which is something I hope to get around to addressing in PyInstaller's multiprocessing hook). And for onefile builds, they also need to set _MEIPASS2 environment variable to prevent the executable from unpacking itself again in the subprocess (using different sys._MEIPASS, and thus breaking the path check in the runtime hook). This is because pyqtgraph.multiprocess uses subprocess.Popen to spawn the process; perhaps we could extend our subprocess runtime hook to detect when sys.executable is spawned and automatically restore _MEIPASS2 in the subprocess' environment?

Replaces and closes #490.

Test that colormap data is available in the frozen application.
Collect all data files from `pyqtgraph` package, except for the
data files that come with the examples.
Add a test for pyqtgraph's RemoteGraphicsView, which makes use
of `pyqtgraph.multiprocess`.
Add a runtime hook that attempts to detect arguments used by
`pyqtgraph.multiprocess` to spawn its subprocess worker.

Requires the user to call `multiprocessing.freeze_support()` in the
entry-point script. Additionally, in onefile builds, the user
needs to set (restore) `_MEIPASS2` environment variable to the
value of `sys._MEIPASS` to prevent the onefile executable from
unpacking  itself again in the subprocess (due to it being spawned
by `pyqtgraph.multiprocess` via `subprocess.Popen()` or equivalent).
@rokm
Copy link
Member Author

rokm commented Oct 19, 2022

Here's oneshot test for the added tests: https://github.com/rokm/pyinstaller-hooks-contrib/actions/runs/3284554698

On linux, the oneshot workflow lacks the apt-installed packages required by PyQt5 and lacks Xvfb setup, hence the failures.

# pyqtgraph.multiprocess also uses a subprocess.Popen() to spawn its
# sub-process, so we need to restore _MEIPASS2 to prevent the executable
# to unpacking itself again in the subprocess.
os.environ['_MEIPASS2'] = sys._MEIPASS
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why this can't go in a runtime hook?

Copy link
Member Author

@rokm rokm Oct 19, 2022

Choose a reason for hiding this comment

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

Yeah, it would potentially cause issues when trying to run a different PyInstaller-frozen executable from the program, which is the main reason why the environment variable is cleared in the first place.

I think the only way to handle this automatically in the safe way is if we manage to patch subprocess.Popen to detect that it is trying to run sys.executable and restore/set _MEIPASS2 in the subprocess' environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it would potentially cause issues when trying to run a different PyInstaller-frozen executable from the program, which is the main reason why the environment variable is cleared in the first place.

Sure, the same problem arises if the user sets the environment variable in their code, but they should know whether their program will also try to spawn a subprocess with a different pyinstaller-frozen executable or not.

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.

2 participants