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

Add a hook for toga #803

Closed
wants to merge 1 commit into from
Closed

Conversation

davetapley
Copy link
Contributor

@davetapley davetapley commented Sep 12, 2024

@rokm
Copy link
Member

rokm commented Sep 12, 2024

Looks like linux runner will require gir1.2-gtk-3.0 and libgirepository1.0-dev to be installed for PyGObject (dependency of toga-gtk).

@@ -0,0 +1 @@
Add a hook for ``toga``, which has metadata.
Copy link
Member

Choose a reason for hiding this comment

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

All packages have metadata. It's really about whether a package feels the need to loop it up at runtime.

Copy link
Member

Choose a reason for hiding this comment

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

Reckon we should add couple of more examples to complement

Add a hook for ``foobar``, which has a hidden import.

Failing with:

Traceback (most recent call last):
  File "toga/icons.py", line 33, in __get__
AttributeError: type object 'Icon' has no attribute '__APP_ICON'. Did you mean: 'APP_ICON'?

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "toga/icons.py", line 126, in __init__
  File "toga/icons.py", line 183, in _full_path
FileNotFoundError: Can't find icon resources/gotcha

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test_source.py", line 8, in <module>
  File "toga/app.py", line 314, in __init__
  File "toga/icons.py", line 35, in __get__
  File "toga/icons.py", line 68, in APP_ICON
  File "toga/icons.py", line 141, in __init__
  File "toga_cocoa/icons.py", line 21, in __init__
  File "pathlib.py", line 1162, in __init__
  File "pathlib.py", line 373, in __init__
TypeError: argument should be a str or an os.PathLike object where __fspath__ returns a str, not 'NoneType'
@davetapley
Copy link
Contributor Author

@rokm @bwoodsend getting to the limit of my knowledge of PyInstaller with this one :|

Couple notes:

  • Maintainers of toga have their own 'make it a binary' tool briefcase, so likely aren't going to be super motivated to work on PyInstaller support.

  • Whole point of toga (and briefcase) is to target different OS as well. I know PyInstaller does too, but probably brings a lot more complexity, I started to tackle that here:

    from PyInstaller.compat import is_darwin, is_linux
    from PyInstaller.utils.hooks import copy_metadata, collect_entry_point
    if is_darwin:
    backend = 'cocoa'
    elif is_linux:
    backend = 'gtk'
    else:
    backend = 'winforms'
    hiddenimports = [f'toga_{backend}', f'toga_{backend}.factory']

I've got pyinstaller-hooks-contrib cloned on a Mac, so targeting that for right now, it uses their cocoa engine.

Failing with:

 File "toga_cocoa/icons.py", line 21, in __init__
  File "pathlib.py", line 1162, in __init__
  File "pathlib.py", line 373, in __init__
TypeError: argument should be a str or an os.PathLike object where __fspath__ returns a str, not 'NoneType'

Probably because we get in to Objective C bindings pretty quick (via this package).

Question: Do you think this will be implementable without a huge amount of effort?

@davetapley davetapley changed the title Add a hook for toga, which has metadata Add a hook for toga Sep 13, 2024
@freakboy3742
Copy link

  • Maintainers of toga have their own 'make it a binary' tool briefcase, so likely aren't going to be super motivated to work on PyInstaller support.

BeeWare maintainer here! You're correct in guessing that working on PyInstaller support isn't high on our list of priorities. However, that doesn't mean we're hostile to PyInstaller. Having more options for installing Toga apps is a good thing, as far as I'm concerned.

To that end - if there's something Toga could do that is preventing it from working with PyInstaller, or there's a change we can make that will make PyInstaller packaging easier, we're not fundamentally opposed to making those changes - especially if that change is based around compliance with existing PEPs. However, it's up to someone else to propose what those changes would be.

@rokm
Copy link
Member

rokm commented Sep 13, 2024

I've opened a new PR (#804) with additional commits on top of yours, because I do not seem to have permissions to push into this one. Will first try to sort out linux and Windows, though, because they seem more straightforward, and then move on to macOS.

@rokm
Copy link
Member

rokm commented Sep 13, 2024

And the problem on macOS is that toga assumes that the frozen/stand-alone bundled application is always an .app bundle (I assume briefcase builds only .app bundles for macOS).

So it tries to retrieve the .app bundles icon by querying CFBundleIconFile key, and passes the result to pathlib.Path without checking if is valid (not None). So this breaks when you build PyInstaller POSIX executable, but works if you build the .app bundle instead.

I think changing these lines from

            bundle_icon = Path(
                NSBundle.mainBundle.objectForInfoDictionaryKey("CFBundleIconFile")
            )
            path = NSBundle.mainBundle.pathForResource(
                bundle_icon.stem,
                ofType=".icns",
            )

to

            bundle_icon = NSBundle.mainBundle.objectForInfoDictionaryKey("CFBundleIconFile")
            if bundle_icon is None:
                # Not an .app bundle (e.g., POSIX build made with PyInstaller)
                raise FileNotFoundError()
            path = NSBundle.mainBundle.pathForResource(
                Path(bundle_icon).stem,
                ofType=".icns",
            )

should fix it. cc @freakboy3742

@freakboy3742
Copy link

freakboy3742 commented Sep 14, 2024

And the problem on macOS is that toga assumes that the frozen/stand-alone bundled application is always an .app bundle (I assume briefcase builds only .app bundles for macOS).

That's correct - and icon handling is one of many reasons for this.

I think changing

Makes sense. PRs welcome :-)

@rokm
Copy link
Member

rokm commented Sep 25, 2024

Superseded by #804.

@rokm rokm closed this Sep 25, 2024
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