search

pdf / kdeconnect-chrome-extension

A browser extension to send pages and content from your browser to connected KDE Connect devices.
https://chrome.google.com/webstore/detail/kde-connect/ofmplbbfigookafjahpeepbggpofdhbo
MIT License
234 stars 30 forks source link

Upgrade godbus dependency to v5.0.6 #43

Closed qwerty12 closed 3 years ago

qwerty12 commented 3 years ago

Updating godbus to v5 is a prerequisite for (haphazardly-done) Windows support. I do not know this change breaks anything on Linux.

Without the v5 import change in dbus.go (which I think is actually needed to use v5), go 1.17.2 on my Windows system builds kdeconnect-chrome-extension against v4,1 which is explicitly built without TCP transport support on Windows.

Unfortunately, glide doesn't handle the change well. If glide.lock needs to be generated from glide,yaml, glide update will fail with: [ERROR] Error scanning github.com\godbus\dbus\v5: cannot find package "." in. I do not know if this is simply because glide hasn't been maintained for years or, not knowing Go, I am missing something. Trying github.com/godbus/dbus/v5 as the package name in glide.yaml doesn't help any.
So I ended up making the changes by hand to glide.lock. glide.yaml has been updated for consistency's sake, though it's not used to actually generate glide.lock here. With glide.lock updated, glide install fetches v5 of godbus without any issues.

(I will mention that glide install fails to create something I can just then go install as per the instructions in the README; I have to run go mod init && go mod vendor after glide install and before go build will work.)

Thanks for the extension.

pdf commented 3 years ago

I should really just migrate to Go modules already. Glad to hear godbus works properly on Windows now, I know some users have been wanting this for some time now.

If all that's required to support Windows is to update to a recent godbus version, I'll try and find some time this week to do the conversion to go.mod and pull it in.

Appreciate you raising this.

ETA: I expect the installer needs some help at a minimum, if you have a second series for Windows stuff, feel free to send it, and we'll rebase/clean up as needed on the way through.

qwerty12 commented 3 years ago

I'll try and find some time this week to do the conversion to go.mod and pull it in.

That would honestly make things easier. I'll close this then.

I expect the installer needs some help at a minimum

Yes, for now, I've just taken to excluding that code from building for a Windows target. The climenu module it uses depends on github.com/pkg/term which refuses to build on Windows.

If you have a second series for Windows stuff, feel free to send it,

This is the "haphazardly-done" part. On Windows, finding out the port KDE Connect's own dbus-daemon is running on requires some creativity. The approach I've gone with is reading the process environment variables of the running kdeconnect-indicator.exe/kdeconnectd.exe in the same Windows session as the native host and copying the contents of its $DBUS_SESSION_BUS_ADDRESS. The code uses pointer arithmetic (there's probably a nice Go way of doing it) and assumes a 64-bit kdeconnect-chrome-extension host process trying to read the values of 64-bit KDE Connect process. The latter isn't a problem in practice because AFAICT official KDE Connect builds are only for 64-bit Windows.

I would like to replace (or supplement?) that with usage of GetExtendedTcpTable. With that, each listening TCP port can be checked to see if if dbus-daemon.exe is the process hosting and if so, an org.freedesktop.DBus.ListNames can be done to see if anything beginning with org.kde.kdeconnect is registered. This is slower, but there's no undocumented code and the bitness doesn't matter. I'll put up another draft PR when/if I've got that working. See https://github.com/pdf/kdeconnect-chrome-extension/pull/44