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

gtk3ui and macos fixes #273

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

Lord-Kamina
Copy link
Contributor

This fixes some standing issues with accelerators on macOS, as well as makes it easier to produce a working app bundle using macports (I made my changes in a new script and bundle-file, in case people prefer jhbuild still; it shouldn't be too hard to mimic what I did for macports to be used with jhbuild)

Of note, it appears in the future it would be desirable to drop GtkosxApplication altogether and use Gtk.Application instead but I'm not sure I am familiar enough with either python nor GTK3 to make those changes myself.

P.S. You can take my .app for a spin here: https://www.dropbox.com/s/zy0y4u41bvp6huk/Deluge.app.zip?dl=1

instead of hardcoded accelerators.
This also fixes accelerator remapping for macOS in Deluge 2.x

Also, use a better syntax for Gdk and Gtk imports. Nothing is gained from importing individual functions or classes (this is even explicitly discouraged for C/C++ programs) and it makes code harder to read.
Set the environment up properly and dump settings under ~/Library instead of ~/.config; while trying to import them from the previous location.

This is more idiomatic for mac and also in theory could allow Deluge 1.3.15 and 2.0.x to coexist.
@Boelensman1
Copy link

Heey Lord-Kamina thanks for your work, hope the deluge team finds some time to look at this soon.

I'm using the .app you provided atm and was wondering if you could also upload the newer build with the deluge-bin fix.

@Lord-Kamina
Copy link
Contributor Author

Lord-Kamina commented Jun 27, 2020

Heey Lord-Kamina thanks for your work, hope the deluge team finds some time to look at this soon.

I'm using the .app you provided atm and was wondering if you could also upload the newer build with the deluge-bin fix.

Will get to it sometime this weekend.

The new build I upload will support handling of magnets (only on macOS 10.13+ though)

This is dependent on a patch  to gtk-mac-integration/GTKOSXApplication (MR already sent upstream)
@Lord-Kamina
Copy link
Contributor Author

Lord-Kamina commented Jul 1, 2020

@Boelensman1 sorry, I got absorbed doing other shit on the weekend. Here you go,

https://www.dropbox.com/s/c51t7y5kh7za2kl/Deluge.app.7z?dl=1

Note, if you're on macOS 10.13+, this should open magnets natively, without relying on the Magnet Handler app. (That functionality cannot be replicated for now though because it depends on an as-yet unmerged PR over at the gtk-mac-integration repo)

@Boelensman1
Copy link

Thanks so much @Lord-Kamina! It works brilliantly

@rnsc
Copy link

rnsc commented Sep 22, 2021

@Lord-Kamina thank you for the updated code, quick question, how can I build it myself on my computer? Is there a documentation I can refer to?

@Lord-Kamina
Copy link
Contributor Author

If you're using macports, you should install all dependencies and then run the make-macports-app.sh (actual name might be slightly different) and then just wait.

@rnsc
Copy link

rnsc commented Sep 23, 2021

@Lord-Kamina thanks, FYI, it doesn't build on Big Sur, some library lookup failing during the setup.py.
More info here: https://www.reddit.com/r/MacOSBeta/comments/hfknpa/is_corefoundation_missing_for_everyone_on_big_sur/

Commenting line 88 and 89 of MacOGraph.py resolves the above issue

#if not os.path.exists(pathname):
  #  raise ValueError("%r does not exist" % (pathname,))

@cas-- cas-- mentioned this pull request Jan 9, 2022
@Lord-Kamina
Copy link
Contributor Author

I've recently updated to Big Sur myself. I haven't tried building it (I have to rebuild dependencies and my dev environment probably) but it's still running at least.

Will take a crack at rebasing master and seeing what the issue is with the build.

Copy link
Member

@cas-- cas-- left a comment

Choose a reason for hiding this comment

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

@Lord-Kamina I did cherry-pick some changes and left comments on a few other items

@@ -534,14 +534,15 @@ def run(self):
]
_package_data['deluge.ui.gtk3'] = ['glade/*.ui']

setup_requires = ['setuptools', 'wheel']
setup_requires = ['setuptools', 'wheel', 'py2app']
Copy link
Member

Choose a reason for hiding this comment

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

py2app should not be requirement as that is macos specific

install_requires = [
'twisted[tls]>=17.1',
# Add pyasn1 for setuptools workaround:
# https://github.com/pypa/setuptools/issues/1510
'pyasn1',
'rencode',
'pyopenssl',
'pygobject',
Copy link
Member

Choose a reason for hiding this comment

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

This is not a requirement for minimal running since this does not apply to deluged.

This does raise the question that do potentially other ui-specific requirements need scrutiny e.g. mako

It could however be included under extra_requires if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not terribly familiar with python setuptools. What would be the preferred way to have this "just work®", adding it under extra_requires as you say? I worked on this quite a long time ago, but if I added this it's most likely because I got an error without it along the way.

Copy link

Choose a reason for hiding this comment

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

Yes, mako is included into install_requires but is WebUI-specific only.

<signal name="activate" handler="on_menuitem_preferences_activate" swapped="no"/>
<accelerator key="P" signal="activate" modifiers="GDK_CONTROL_MASK"/>
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer if these accelerator keys were not removed from the glade files and instead we found an alternative way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's possible to implement portable shortcuts without removing them from the glade files.

@endercypher
Copy link

Hey, know this is an old issue. Is there a possibility of the app being updated to work on macOS Sonoma? There seems to be an issue with the python executable & the python path not being set after updating. Thanks in advance

@matthieuki
Copy link

Hi @Lord-Kamina, Do you think is it possible to do an update for Sonoma ? Thank you 😊

@endercypher
Copy link

Hi @Lord-Kamina, Do you think is it possible to do an update for Sonoma ? Thank you 😊

id compile it myself but I'm running into this issue.

Traceback (most recent call last):
File "/Users/Ender/Documents/Deluge-builds/delugenew/setup.py", line 564, in
setup(
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/init.py", line 107, in setup
return distutils.core.setup(**attrs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 185, in setup
return run_commands(dist)
^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 201, in run_commands
dist.run_commands()
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 969, in run_commands
self.run_command(cmd)
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/dist.py", line 1233, in run_command
super().run_command(command)
File "/opt/homebrew/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
cmd_obj.run()
File "/opt/homebrew/lib/python3.11/site-packages/py2app/build_app.py", line 984, in run
self._run()
File "/opt/homebrew/lib/python3.11/site-packages/py2app/build_app.py", line 1214, in _run
self.run_normal()
File "/opt/homebrew/lib/python3.11/site-packages/py2app/build_app.py", line 1327, in run_normal
self.create_binaries(py_files, pkgdirs, extensions, loader_files)
File "/opt/homebrew/lib/python3.11/site-packages/py2app/build_app.py", line 1774, in create_binaries
mm.mm.run_file(fmwk)
File "/opt/homebrew/lib/python3.11/site-packages/macholib/MachOGraph.py", line 89, in run_file
raise ValueError("%r does not exist" % (pathname,))
ValueError: '/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation' does not exist

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.

7 participants