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 PyInstaller for freeze test env #1773

Merged
merged 2 commits into from
Jul 27, 2016

Conversation

nicoddemus
Copy link
Member

cx_freeze doesn't seem to be very well supported in Python 3.5.

Using PyInstaller instead and rename environment to "freeze" which
is a more generic term for freezing python code into standalone
executables. PyInstaller is also much simpler to use.

Also note that this did not change any functionality, only internal testing and docs, that's why I'm targeting master here.

Fix #1769

@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Coverage increased (+0.01%) to 92.316% when pulling 7a0adfc on nicoddemus:fix-freeze into d911bfc on pytest-dev:master.

@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Coverage increased (+0.01%) to 92.316% when pulling 2fc35ff on nicoddemus:fix-freeze into d911bfc on pytest-dev:master.

cx_freeze doesn't seem to be very well supported in Python 3.5.

Using pyinstaller instead and rename environment to "freeze" which
is a more generic term for freezing python code into standalone
executables.

Fix pytest-dev#1769
@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Coverage increased (+0.01%) to 92.316% when pulling ed36d62 on nicoddemus:fix-freeze into d911bfc on pytest-dev:master.

@nicoddemus
Copy link
Member Author

As a note, opened pyinstaller/pyinstaller#2119 so the user doesn't even need to use the freeze_includes function with PyInstaller. 😁

@The-Compiler
Copy link
Member

Hmm, seems like there are some warnings on AppVeyor:

9936 WARNING: Can not get binary dependencies for file: C:\windows\system32\api-ms-win-crt-utility-l1-1-0.dll
Traceback (most recent call last):
  File "c:\projects\pytest\.tox\freeze\lib\site-packages\PyInstaller\depend\bindepend.py", line 695, in getImports
    return _getImports_pe(pth)
  File "c:\projects\pytest\.tox\freeze\lib\site-packages\PyInstaller\depend\bindepend.py", line 122, in _getImports_pe
    dll, _ = sym.forwarder.split('.')
TypeError: a bytes-like object is required, not 'str'

But it seems to work just fine. PyInstaller bug?

for x in pytest.freeze_includes():
hidden.extend(['--hidden-import', x])
args = ['pyinstaller'] + hidden + ['runtests_script.py']
subprocess.check_call(' '.join(args), shell=True)
Copy link
Member

Choose a reason for hiding this comment

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

Why not simply subprocess.check_call(args) without shell=True? Also, it might be better to tell users what to add to their spec file as that's (afaik) how PyInstaller is usually used for non-trivial things (e.g. I have this file for qutebrowser)

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not simply subprocess.check_call(args) without shell=True?

On Windows without shell=True you have to add the .exe extension:

>>> import subprocess
>>> subprocess.check_call('py.test --help')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "e:\ws\Souring\envs\steam_souring10\lib\subprocess.py", line 536, in check_call
    retcode = call(*popenargs, **kwargs)
  File "e:\ws\Souring\envs\steam_souring10\lib\subprocess.py", line 523, in call
    return Popen(*popenargs, **kwargs).wait()
  File "e:\ws\Souring\envs\steam_souring10\lib\subprocess.py", line 711, in __init__
    errread, errwrite)
  File "e:\ws\Souring\envs\steam_souring10\lib\subprocess.py", line 959, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified
>>> subprocess.check_call('py.test.exe --help', shell=True)
... <output> ...
0

And I wanted to keep things simple here.

Also, it might be better to tell users what to add to their spec file ...

My intention in this topic is not actually be a complete help on how to use PyInstaller, only to briefly demonstrate how to use the pytest.freeze_includes() function. Actually now that pyinstaller/pyinstaller#2119 is merged, I think this docs should be updated to just mention that pytest should work out of the box with the latest PyInstaller, while others using other tools can use the freeze_includes() function to obtain the list of pytest internal modules.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

On Windows without shell=True you have to add the .exe extension:

Oh wow. I guess passing a list (['py.test', '--help']) doesn't help either?

Actually now that pyinstaller/pyinstaller#2119 is merged, I think this docs should be updated to just mention that pytest should work out of the box with the latest PyInstaller, while others using other tools can use the freeze_includes() function to obtain the list of pytest internal modules.

Sounds good to me!

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wow. I guess passing a list (['py.test', '--help']) doesn't help either?

No, the problem is there's no py.test file, only py.test.exe... shell=True works because the shell let's you get away with typing the .exe yourself. 😉

Sounds good to me!

Updated. 😁

@codewarrior0
Copy link
Contributor

But it seems to work just fine. PyInstaller bug?

Yep. It's fixed for the next release.

(It's related to us switching the pefile library from an unofficial python3 port to the official one.)

@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Coverage increased (+0.01%) to 92.316% when pulling 10ebc0c on nicoddemus:fix-freeze into d911bfc on pytest-dev:master.


Fortunately recent ``PyInstaller`` releases already have a custom hook
for pytest, but if you are using another tool to freeze executables
such as ``cx_freeze`` or ``py2exe``, you can use the ``pytest.freeze_includes()``
Copy link
Member

Choose a reason for hiding this comment

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

no "the" here 😉

As discussed during the review, suggest in general
to use PyInstaller and just mention pytest.freeze_includes()
in less detail on how to actually use it, because it varies
from tool to tool.
@coveralls
Copy link

coveralls commented Jul 27, 2016

Coverage Status

Coverage remained the same at 92.316% when pulling ae9d3bf on nicoddemus:fix-freeze into 7b15206 on pytest-dev:master.

@The-Compiler
Copy link
Member

LGTM, will merge once the CI passes.

@The-Compiler The-Compiler merged commit ffb583a into pytest-dev:master Jul 27, 2016
@The-Compiler
Copy link
Member

Thanks!

@nicoddemus
Copy link
Member Author

👍 😄

@nicoddemus nicoddemus deleted the fix-freeze branch July 27, 2016 13:07
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.

4 participants