search

Digital-Sapphire / PyUpdater

Pyinstaller auto-update library
https://www.pyupdater.org
460 stars 92 forks source link

PyInstaller hook conflict #288

Closed colin-fsa closed 2 years ago

colin-fsa commented 3 years ago

I believe that there is a conflict between PyUpdater v4 and PyInstaller v3. Starting with PyInstaller v3.1 there is an nacl hook included which wasn't removed until PyInstaller v4. This causes a conflict with PyUpdater v4 which also has an nacl hook.

The duplicate hook causes an assertion error when attempting to build the exe. The solution is to upgrade PyInstaller to v4, so I believe the systematic fix is to updates requirements.txt to require pyinstaller>=4.0.

If updating the requirements file is truly all that's needed I'd be happy to submit a PR with the change.

A minimal example is below, trying to build any python script with this environment will result in: _File "c:...\env\lib\site-packages\PyInstaller\depend\imphook.py", line 289, in init assert self.hook_module_name not in HOOKS_MODULENAMES AssertionError requirements.txt main.zip

JMSwag commented 3 years ago

Hi @cscheriff, thanks for the report. I think we could also remove the ncal hook from PyUpdater. I don't want to force PyInstaller v4 on anyone. A PR is welcomed ;)

JessicaTegner commented 3 years ago

Just curious for your decision here @JMSwag Why wouldn't you have PyUpdater Rely on PyInstaller >= 4.0 I mean, new is better right? And besides, wouldn't there be some speed / security improvements gained?

JMSwag commented 3 years ago

@NicklasTegner Essentially, I want to give end users the ability to decide. If this repo housed an application and not a library, I'd want to be using the latest and greatest.

colin-fsa commented 3 years ago

@JMSwag Sorry, I meant to write back to clarify but dropped the ball. Are you saying the nacl hook that's in PyUpdater isn't actually needed? Since it was a change in the latest version I assumed there was a still-relevant reason it was added.

JMSwag commented 3 years ago

@cscheriff I'm pretty sure it's not necessary. I can confirm later tonight.