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

Bootloader: OSX: Respect Info.plist options #3566

Merged
merged 2 commits into from Jul 7, 2018

Conversation

Projects
None yet
3 participants
@BoboTiG
Contributor

BoboTiG commented Jun 11, 2018

A possible fix for #1917.

What is the fix/workaround?

As proposed by @codewarrior0 in his comment, I added a bootloader runtime option pyi-osx-background that triggers the right transform type in pyi_launch_initialize().

This is the simplest option as it does not require to add new bootloaders nor a new argument to handle this specific case. The behavior is restored as it should have been: when the user sets LSUIElement in the Info.plist dict, PyInstaller handles it.

Current status

The patch is good but does not work automatically because the TOC option is not yet forwarded to the bootloader.

In $project/build/BUNDLE-00.toc, I have the line ('pyi-osx-background', '', 'OPTION') setted, which is what we need.
But as this kind of TOC line is traited before (when building the PKG), it is not possible to get it via pyi_arch_get_option(status, "pyi-osx-background"), steps are:

289 INFO: PyInstaller: 3.4.dev0+g8c071568.mod
290 INFO: Python: 3.6.2
297 INFO: Platform: Darwin-16.7.0-x86_64-i386-64bit
299 INFO: UPX is not available.
301 INFO: Extending PYTHONPATH with paths
['.../pyinstaller']
301 INFO: checking Analysis
301 INFO: Building Analysis because Analysis-00.toc is non existent
302 INFO: Initializing module dependency graph...
303 INFO: Initializing module graph hooks...
305 INFO: Analyzing base_library.zip ...
4519 INFO: running Analysis Analysis-00.toc
4533 INFO: Caching module hooks...
4538 INFO: Analyzing systray.py
4552 INFO: Loading module hooks...
4553 INFO: Loading module hook "hook-encodings.py"...
4627 INFO: Loading module hook "hook-pydoc.py"...
4627 INFO: Loading module hook "hook-PyQt5.py"...
4657 INFO: Loading module hook "hook-PyQt5.QtCore.py"...
4749 INFO: Loading module hook "hook-PyQt5.QtGui.py"...
4784 INFO: Loading module hook "hook-PyQt5.QtWidgets.py"...
4819 INFO: Loading module hook "hook-xml.py"...
5172 INFO: Looking for ctypes DLLs
5172 INFO: Analyzing run-time hooks ...
5177 INFO: Including run-time hook 'pyi_rth_qt5.py'
5189 INFO: Looking for dynamic libraries
5451 INFO: Looking for eggs
5451 INFO: Using Python library venv-pyinstaller/bin/../.Python
5457 INFO: Warnings written to systray/build/systray/warn-systray.txt
5495 INFO: Graph cross-reference written to systray/build/systray/xref-systray.html
5508 INFO: checking PYZ
5508 INFO: Building PYZ because PYZ-00.toc is non existent
5509 INFO: Building PYZ (ZlibArchive) systray/build/systray/PYZ-00.pyz
5887 INFO: Building PYZ (ZlibArchive) systray/build/systray/PYZ-00.pyz completed successfully.
5894 INFO: checking PKG
5894 INFO: Building PKG because PKG-00.toc is non existent
5894 INFO: Building PKG (CArchive) PKG-00.pkg
5904 INFO: Building PKG (CArchive) PKG-00.pkg completed successfully.
5905 INFO: Bootloader pyinstaller/PyInstaller/bootloader/Darwin-64bit/runw_d
5905 INFO: checking EXE
5905 INFO: Building EXE because EXE-00.toc is non existent
5905 INFO: Building EXE from EXE-00.toc
5905 INFO: Appending archive to EXE systray/build/systray/systray
5907 INFO: Fixing EXE for code signing systray/build/systray/systray
5915 INFO: Building EXE from EXE-00.toc completed successfully.
5917 INFO: checking COLLECT
5917 INFO: Building COLLECT because COLLECT-00.toc is non existent
5917 INFO: Building COLLECT COLLECT-00.toc
7202 INFO: Building COLLECT COLLECT-00.toc completed successfully.
7209 INFO: This is an agent app (LSUIElement is set to True)
7209 INFO: checking BUNDLE
7209 INFO: Building BUNDLE because BUNDLE-00.toc is non existent
7209 INFO: Building BUNDLE BUNDLE-00.toc
8430 INFO: moving BUNDLE data files to Resource directory

As we can see, the line 7209 INFO: This is an agent app (LSUIElement is set to True) appears too late in the process.

Any clue to make it easy forwarded? Perhaps I misunderstand something?

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch 5 times, most recently from b9b9f28 to af537be Jun 11, 2018

@htgoebel

All of these options have to be present when the PKG is build (which is triggered by the EXE). So I'm afraid we need to add another keyword argument to EXE, like it is done in a582974.

To avoid the need for the developer to handle this twice, you could add an entry to self.info_plist around here. If info_plist contains a contradicting entry, raise an error.

I also marked some code issues already.

@@ -163,6 +168,8 @@ def assemble(self):
links = []
toc = add_suffix_to_extensions(self.toc)
for inm, fnm, typ in toc:
if typ == 'OPTION':
continue

This comment has been minimized.

@htgoebel

htgoebel Jun 11, 2018

Member

This is an unrelated change and shall go into a commit of it's own.

This comment has been minimized.

@BoboTiG

BoboTiG Jun 12, 2018

Contributor

I had to add this to prevent such exception:

8090 INFO: Building BUNDLE BUNDLE-00.toc
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "pyinstaller/pyinstaller.py", line 15, in <module>
    run()
  File "pyinstaller/PyInstaller/__main__.py", line 109, in run
    run_build(pyi_config, spec_file, **vars(args))
  File "pyinstaller/PyInstaller/__main__.py", line 61, in run_build
    PyInstaller.building.build_main.main(pyi_config, spec_file, **kwargs)
  File "pyinstaller/PyInstaller/building/build_main.py", line 838, in main
    build(specfile, kw.get('distpath'), kw.get('workpath'), kw.get('clean_build'))
  File "pyinstaller/PyInstaller/building/build_main.py", line 784, in build
    exec(text, spec_namespace)
  File "<string>", line 45, in <module>
  File "pyinstaller/PyInstaller/building/osx.py", line 91, in __init__
    self.__postinit__()
  File "pyinstaller/PyInstaller/building/datastruct.py", line 158, in __postinit__
    self.assemble()
  File "pyinstaller/PyInstaller/building/osx.py", line 182, in assemble
    shutil.copy(fnm, tofnm)
  File "/lib/python3.6/shutil.py", line 241, in copy
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/lib/python3.6/shutil.py", line 120, in copyfile
    with open(src, 'rb') as fsrc:
FileNotFoundError: [Errno 2] No such file or directory: ''

But I will remove it because it will become useless after the next round.

@@ -53,7 +53,12 @@ def __init__(self, *args, **kws):
# Fallback to appname.
self.bundle_identifier = self.appname
self.info_plist = kws.get('info_plist', None)
self.info_plist = kws.get('info_plist', [])

This comment has been minimized.

@htgoebel

htgoebel Jun 11, 2018

Member

If info_plist is not given as an argument, self.info_plist will become a list, and the next line will fail:

>>> [].get('a')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'list' object has no attribute 'get'

This comment has been minimized.

@BoboTiG

BoboTiG Jun 12, 2018

Contributor

Yeah, stupid mistake. 👍

@BoboTiG BoboTiG changed the title from MacOS: Respect the LSUIElement=1 plist option to [WIP] MacOS: Respect the LSUIElement=1 plist option Jun 12, 2018

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch 4 times, most recently from 94ddf9c to 8de5a88 Jun 12, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 12, 2018

I updated the PR, it is working now.

I don't know if I had to add the --osx-background CLI argument. I would say it is not very useful, WDYT?

Also, should I include updated bootloaders?

Let me know your thoughts, do not hesitate to ask for minor changes (even in logged sentences). When it will be OK, I will add documentation and test cases.

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch from 4b26789 to 2d25a68 Jun 12, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 12, 2018

Finally I found a way simpler fix: by not calling ourselves TransformProcessType(), the OS handles it automatically.
I just tested several scenarii and all worked well:

  • Without Info.plist: display normally with icon in the Dock.
  • With Info.plist LSUIElement=True: display normally with no icon in the Dock.
  • With Info.plist LSBackgroundOnly=True: no display nor icon in the Dock.

I think we have the desired behavior. Same results with console=True, Info.plist options are taken into account.

I am open to any advice/review to ensure I did not break something.
Is there someone who can test on older versions like macOS 10.9?

For testing, you can use these files:

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jun 12, 2018

Great, looks like you found the root cause. :-)

I wonder if console=True should set LSUIElement or LSBackgroundOnly in the info_plist? Or should we leave this to the developer? If we leave this to the developer, shall we add a description to the manual?

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 12, 2018

I think it should automatically set LSBackgroundOnly (as stated in the documentation, it seems the right thing to do).

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch from 2d25a68 to 75107ae Jun 12, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 12, 2018

Done in a separate commit. Do you prefer all in one commit?

Also, what about updated bootloaders? Should I provide them in a new commit?

@@ -354,6 +354,9 @@ def __init__(self, *args, **kwargs):
else:
self.upx = False
# Save the console value for later use on macOS
CONF['console'] = self.console

This comment has been minimized.

@htgoebel

htgoebel Jun 12, 2018

Member

No need for this. The console can be picked up in EXE at https://github.com/BoboTiG/pyinstaller/blob/v3.3.1/PyInstaller/building/osx.py#L62. And when doing this, console can be per EXE, while when storing the value in CONFIG it's global for all executable.

This comment has been minimized.

@BoboTiG

BoboTiG Jun 12, 2018

Contributor

I would prefer too, but condition if isinstance(arg, EXE): is never taken. BUNDLE is working with COLLECT only, not EXE.

This comment has been minimized.

@htgoebel

htgoebel Jun 12, 2018

Member

IC. According to the templates (PyInstaller/building/templates.py), only for a one-file BUNDLE, the first argument is an EXE. For the one-dir Bundle, it is a COLLECT.

Two possible solutions:

  1. In COLLECT there is already a loop scanning for EXE. Use this to set COLLECT's .console for the collection. This has the drawback, that we hide "a bundle can contain only one exe" in an unrelated area of the code.
  2. In BUNDLE, if it is a COLLECT, scan the COLLECT's .toc for an exe and fetch the info from there. I just discover that there is already a loop searching for Exe. This loop takes into account the first exe only, thus I would do the same for .console.

This comment has been minimized.

@BoboTiG

BoboTiG Jun 13, 2018

Contributor

The option 2 is not doable (as of now) because data in toc is just strings, for instance something like:

('systray', '.../systray/build/systray/systray', 'EXECUTABLE')

If we have had an attribute pointing to the EXE object it would have been easy. But I do not think it is a good thing to do that.

I updated the PR to remove the use of CONF using option 1.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Jun 12, 2018

I would prefer to have this two commits: One for the Bootloader change and one for "Automatically set Info.plist LSBackgroundOnly=True when consol…".

Please do not build bootloaders, we have the policy that only bootloaders compiled by core develeopers are commited. And we need to recompile all of then soon anyway due to the upcoming Python 3.7 release.

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch from 132ae78 to 43ade69 Jun 13, 2018

@BoboTiG BoboTiG changed the title from [WIP] MacOS: Respect the LSUIElement=1 plist option to MacOS: Respect Info.plist options Jun 14, 2018

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 14, 2018

Do you need changes on the PR?

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch from 43ade69 to b387398 Jun 15, 2018

@BoboTiG BoboTiG changed the title from MacOS: Respect Info.plist options to Bootloader: OSX: Respect Info.plist options Jun 15, 2018

@micahflee

This comment has been minimized.

micahflee commented Jun 22, 2018

When this PR is merged, will it totally fix #1917? I'm looking forward to it!

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 22, 2018

It should do the trick. Do you think you can check before any merging?

@micahflee

This comment has been minimized.

micahflee commented Jun 25, 2018

Yes I'll give it a shot.

@micahflee

This comment has been minimized.

micahflee commented Jun 25, 2018

I just tested myself, and it appears to work great! I also build my own bootloader, not sure if that was necessary or not. I can't wait for the next PyInstaller release.

@micahflee micahflee referenced this pull request Jun 25, 2018

Merged

Hide dock icon #124

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jun 26, 2018

Great!

Yes, it is required to rebuild bootloaders in order to make the patch working.

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Jun 27, 2018

@htgoebel htgoebel added the bootloader label Jun 27, 2018

@htgoebel htgoebel self-assigned this Jun 27, 2018

@htgoebel

Thanks for the update. Looks good to me. Only one small suggestion.

}
# Merge info_plist settings from spec file
if isinstance(self.info_plist, dict) and self.info_plist:
info_plist_dict.update(self.info_plist)
# Setting EXE console=True implicitely sets LSBackgroundOnly to True
if self.console:
info_plist_dict['LSBackgroundOnly'] = True

This comment has been minimized.

@htgoebel

htgoebel Jul 6, 2018

Member

If we move this in front of "Merge info_plist settings from spec file", this would allow users to overwrite the LSBackgroundOnly if needed for some reason. What do you think?

This comment has been minimized.

@BoboTiG

BoboTiG Jul 6, 2018

Contributor

Nice catch, change applied.

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch 3 times, most recently from 6508b60 to 1c55ae5 Jul 6, 2018

@BoboTiG BoboTiG force-pushed the BoboTiG:fix-mac-ui-element branch from 1c55ae5 to f7aa706 Jul 6, 2018

htgoebel added a commit that referenced this pull request Jul 7, 2018

Bootloader: OSX: Don't make it a GUI app, even in windowed-mode.
Not enforcing this programmatically in the bootloader allows to
controll behaviour using Info.plist options - which can by set in
PyInstaller itself.

- Without Info.plist: display normally with icon in the Dock.
- With Info.plist LSUIElement=True: display normally with no icon in
  the Dock.
- With Info.plist LSBackgroundOnly=True: no display nor icon in the
  Dock.

See gh-1917 and gh-3566.

@htgoebel htgoebel merged commit 5c2fe6c into pyinstaller:develop Jul 7, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@htgoebel

This comment has been minimized.

Member

htgoebel commented Jul 7, 2018

Thanks you very much for this nice pull-request and for finding this simple solution!

@BoboTiG

This comment has been minimized.

Contributor

BoboTiG commented Jul 7, 2018

And thanks for your patience and supervision.
Pleasure to contribute, I will keep going.

@BoboTiG BoboTiG deleted the BoboTiG:fix-mac-ui-element branch Jul 7, 2018

cowo78 pushed a commit to cowo78/pyinstaller that referenced this pull request Dec 7, 2018

Bootloader: OSX: Don't make it a GUI app, even in windowed-mode.
Not enforcing this programmatically in the bootloader allows to
controll behaviour using Info.plist options - which can by set in
PyInstaller itself.

- Without Info.plist: display normally with icon in the Dock.
- With Info.plist LSUIElement=True: display normally with no icon in
  the Dock.
- With Info.plist LSBackgroundOnly=True: no display nor icon in the
  Dock.

See pyinstallergh-1917 and pyinstallergh-3566.

cowo78 pushed a commit to cowo78/pyinstaller that referenced this pull request Dec 7, 2018

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