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

geniuspaste: Port to libsoup3 (and avoid GTK3 deprecations) #1342

Merged
merged 9 commits into from
Jun 16, 2024

Conversation

b4n
Copy link
Member

@b4n b4n commented Apr 26, 2024

Many pastebins don't work, but it's not new to the changes here…

CC @xiota @jbicha

@b4n b4n requested a review from elextr April 26, 2024 21:26
Copy link
Member

@elextr elextr left a comment

Choose a reason for hiding this comment

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

Bearing in mind my lack of knowledge of libsoup it looks ok.

@xiota
Copy link
Contributor

xiota commented Apr 27, 2024

Tested... Seems to work.

@b4n
Copy link
Member Author

b4n commented Apr 27, 2024

Thanks for review & testing!

BTW, a few things things to note:

  • libsoup3 is fairly recent (2021-09-18), and e.g. not available in Debian before Bookworm (12, current stable)
  • I probably won't make it possible to support both libsoup2 and libsoup3, because the changes are too complex which would make the code somewhat of a nightmare.
  • This means that other plugins have to be compiled against e.g. webkit2gtk 4.1, not 4.0.

Is that OK with everybody? @elextr @frlan It is with me, I feel it's OK for the plugin to live in the present, and with the webkit question we don't really have a choice (apart from process isolation). Also, maybe this means we want to drop support for webkit2gtk 4.0 in the other plugins, or have a fancy check in the build system to try and avoid the case where different plugins use incompatible libraries -- but that's gonna be non-trivial, and probably not worth it.

Also a subsequent improvement would be to use the asynchronous libsoup API, because now it blocks the UI. Before this PR, it looked a bit better because IIUC (not verified) one of the libsoup calls spun the main loop manually, but this likely had some unexpected side effects of its own. I will try not to include this here.

@xiota
Copy link
Contributor

xiota commented Apr 27, 2024

not available in Debian before Bookworm (12, current stable)

Do Debian and Ubuntu even bother rebuilding packages for old releases?

After testing the plugin, I think it should be dropped. Most of the pastebins don't work. There's no confirmation of what it's sending. Half the time, there's no way to view the pastebin after it's sent because plugin failed to open browser on my computer, and didn't print any message indicating issue, or where to get pastebin, etc. (The other half, were error popups.) Once data is sent, there's no way to delete it, which is a security risk if accidentally (or maliciously) triggered.

The pastebin that did work (https://pastebin.geany.org/) hadn't been used since Dec 2022. So the plugin probably isn't being used.

@elextr
Copy link
Member

elextr commented Apr 27, 2024

Do Debian and Ubuntu even bother rebuilding packages for old releases?

Bugfixes, security fixes and those sorts of things, but rarely new versions.

To be honest @xiota has a point, if the plugin causes constraints on others we need to look at what value it adds, and if it only works for one pastebin and what it replaces is ctrl+a ctrl+c move to the web entry of the pastebin ctrl+v I don't see it is adding a huge lot of value. The original creator has not done anything with it for 12 years, its been others doing bugfixes and infrastructure changes to keep it alive. Maybe it is time to give it a viking funeral and consign it to the depths of git (from which anyone interested can revive it if they want to).

@b4n
Copy link
Member Author

b4n commented Apr 27, 2024

After testing the plugin, I think it should be dropped. Most of the pastebins don't work.

Indeed only 3 did in my tests (Geany's, Debian's and Sprunge)

There's no confirmation of what it's sending. […] Once data is sent, there's no way to delete it, which is a security risk if accidentally (or maliciously) triggered.

It's indeed unfortunate that it doesn't report deletion URL for pastebins that support it.

Half the time, there's no way to view the pastebin after it's sent because plugin failed to open browser on my computer, and didn't print any message indicating issue, or where to get pastebin, etc. (The other half, were error popups.)

Failing to open the browser is odd, because it uses Geany's own utils_open_browser(), so that suggests this doesn't work for you either?
Though, note that the default behavior is to show the URL, not open it.

The pastebin that did work (https://pastebin.geany.org/) hadn't been used since Dec 2022. So the plugin probably isn't being used.

That part is not true: it's just that by default the pastes are deleted after a while, and it just means that nobody had made a forever-paste since then.
Also, https://paste.debian.net works, albeit not for single-line pastes (but indeed the plugin fails to report the error).
It's not to say that the plugin is used though, it might not be.


Anyway, as @elextr said this has been abandoned by it's author, and I've been surprised of the amount of work I did put in it myself over the years (not that I really mind, but I don't think of myself as the maintainer).

I think that this plugin has some occasional use (although I don't myself, and it lacks support for some popular ones like gists), but I won't weep over its coffin if that should be its fate. Also, there are some competent tools around that might be a worthy replacement, like pastebinit that could probably be wrapped to offer a similar featureset.

Pastebin service support might however be easy enough to improve; it's configuration files that need to be added/removed/adapted to pastebin service changes.

@b4n
Copy link
Member Author

b4n commented Apr 27, 2024

Do Debian and Ubuntu even bother rebuilding packages for old releases?

No, the dependency only matter for users of older releases that would like to build a newer Geany by themselves. And Debian was just an example, sometimes we check what even more slow-paced distro do, like RHEL or so.
Anyway, I don't think this should be a reason to go one direction or another myself, as staying with libsoup2 is causing issues on current and future distros, and again I won't add support for selecting the one to use myself.

@xiota
Copy link
Contributor

xiota commented Apr 27, 2024

it uses Geany's own utils_open_browser(), so that suggests this doesn't work for you either?

I had seen the browser setting, but don't know what actually uses it. Same with the terminal command. What actually uses it? I also often have ... "send selection to" not working until I restart geany. I'm close to being annoyed enough with it to search for the problem.

Though, note that the default behavior is to show the URL, not open it.

It didn't appear to do either. No popup or message in status or msgwin on success. I don't recall seeing anything but some usual gtk errors in terminal.

... by default the pastes are deleted after a while, and it just means that nobody had made a forever-paste since then.

I don't see how to use the geany pastebin service without the plugin. There's no web form I can find.

@b4n
Copy link
Member Author

b4n commented Apr 27, 2024

It didn't appear to do either. No popup or message in status or msgwin on success. I don't recall seeing anything but some usual gtk errors in terminal.

Weird… unless you have a libsoup2 plugin loaded? IIUC nothing will stop you from doing so, but things will just perform bizarrely (or crashingly). Or maybe the plugin has more issues than I can see myself.

... by default the pastes are deleted after a while, and it just means that nobody had made a forever-paste since then.

I don't see how to use the geany pastebin service without the plugin. There's no web form I can find.

Ah, indeed, it was removed due to spam… quoting @eht16 from an internal mail:

I removed the form to create new snippets and related code [due to recurring spam].
Only the API endpoint is still available for the GeniusPaste plugin.

So indeed, only the plugin (or another API-based tool, like an oooold pastebinit configuration I have laying around).
Anyway, that doesn't change much on whether this says anything to whether the plugin is in use or not, as the snippets expire anyway.

@@ -3,7 +3,8 @@ AC_DEFUN([GP_CHECK_GENIUSPASTE],
GP_ARG_DISABLE([GeniusPaste], [auto])

GP_CHECK_PLUGIN_DEPS([GeniusPaste], GENIUSPASTE,
[libsoup-2.4 >= 2.4.0])
[gtk+-3.0 >= 3.16
Copy link
Member

Choose a reason for hiding this comment

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

We also need to adjust the CI workflow config to install libsoup3. Currently the plugin will not be built in the CI due to the missing dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, trying it now -- but that also requires bumping the Ubuntu version because 20.04 doesn't have it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@eht16 Seems to work, but that hit the cppcheck issue again, do you know anything about that?

Copy link
Member

Choose a reason for hiding this comment

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

No. It breaks also the nightly builds and so I merged #1310 today to unhide this problem.
Unfortunately, now it is not hidden any longer.

@b4n b4n force-pushed the geniuspaste-libsoup3 branch 3 times, most recently from 0e025a7 to d336856 Compare April 28, 2024 19:54
@eht16
Copy link
Member

eht16 commented May 4, 2024

I tried to test it on Windows but got mixed results :(.

First, we need more dependencies on Windows:

diff --git a/build/gtk-bundle-from-msys2.sh b/build/gtk-bundle-from-msys2.sh
index 408d727f..529fa24a 100644
--- a/build/gtk-bundle-from-msys2.sh
+++ b/build/gtk-bundle-from-msys2.sh
@@ -21,7 +21,7 @@ EXE_WRAPPER_64="mingw-w64-x86_64-wine"
 # enchant, hunspell - for SpellCheck plugin
 # lua51 - for GeanyLua plugin
 # gnupg, gpgme - for GeanyPG plugin
-# libsoup - for UpdateChecker plugin
+# libsoup3 - for UpdateChecker & GeniusPaste plugins
 # libgit2 - for GitChangeBar plugin
 # gtkspell3 - for GeanyVC plugin
 # the rest is dependency-dependency
@@ -30,6 +30,7 @@ ca-certificates
 ctags
 ctpl-git
 enchant
+glib-networking
 gnupg
 gpgme
 http-parser
@@ -39,8 +40,9 @@ libgcrypt
 libgit2
 libgpg-error
 libidn2
+libproxy
 libpsl
-libsoup
+libsoup3
 libssh2
 libsystre
 libunistring

(The diff probably won't apply cleanly against this PR but should be easy enough to adopt, sorry.)
It is important to remove "libsoup" as it cannot be installed together with "libsoup3". There is a runtime warning when both libraries are loaded.
This obviously has a direct impact on the UpdateChecker plugin, so we probably need to merge them in order and rebase the latter one after the first one is merged.

Then, I got a compiler warning which might or might not be new but still worth to look at:

In file included from C:/msys64/mingw64/include/glib-2.0/glib/giochannel.h:36,
                 from C:/msys64/mingw64/include/glib-2.0/glib.h:56,
                 from C:/msys64/mingw64/include/glib-2.0/gobject/gbinding.h:30,
                 from C:/msys64/mingw64/include/glib-2.0/glib-object.h:24,
                 from C:/msys64/mingw64/include/glib-2.0/gio/gioenums.h:30,
                 from C:/msys64/mingw64/include/glib-2.0/gio/giotypes.h:30,
                 from C:/msys64/mingw64/include/glib-2.0/gio/gio.h:28,
                 from C:/msys64/mingw64/include/libsoup-3.0/libsoup/soup-types.h:9,
                 from C:/msys64/mingw64/include/libsoup-3.0/libsoup/soup-auth.h:8,
                 from C:/msys64/mingw64/include/libsoup-3.0/libsoup/soup.h:11,
                 from geniuspaste.c:22:
geniuspaste.c: In function 'json_request_new':
C:/msys64/mingw64/include/glib-2.0/glib/gstring.h:74:5: warning: ignoring return value of 'g_string_free_and_steal' declared with attribute 'warn_unused_result' [-Wunused-result]
   70 |   (__builtin_constant_p (free_segment) ?        \
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   71 |     ((free_segment) ?                           \
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |       (g_string_free) ((str), (free_segment)) : \
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |       g_string_free_and_steal (str))            \
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   74 |     :                                           \
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   75 |     (g_string_free) ((str), (free_segment)))
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
geniuspaste.c:612:5: note: in expansion of macro 'g_string_free'
  612 |     g_string_free(str, FALSE);
      |     ^~~~~~~~~~~~~

When trying to paste some file, Geany crashes for me when it is executed natively. I don't know yet why this happens and I didn't manage it yet to debug it with gdb. This might be related to my system where I use some firewall rules to block unwanted internet access from Windows and also have proxy connections configured and after all, it is Windows 7.

When I start Geany from within the MSYS2 environment, pasting some file works to some extend, at least no crash :).
But the response look weird:
Screenshot_2024-05-04_12-48-33
The link below "\1" is actually also "\1".

The response_str in pastebin_parse_response is "https://www.geany.org/p/6vzVo" which is correct. So there seems to be a problem with parsing the URL. I don't know if this related to these changes or not.

Anyway, with the bundle changes above, we should get usable installers from the CI and so others could test it as well with a less weird Windows setup :).

@b4n
Copy link
Member Author

b4n commented May 15, 2024

Then, I got a compiler warning which might or might not be new but still worth to look at:

[…]

It's actually not an issue (the buffer is taken the line before), but probably worth silencing.

When trying to paste some file, Geany crashes for me when it is executed natively. I don't know yet why this happens and I didn't manage it yet to debug it with gdb. This might be related to my system where I use some firewall rules to block unwanted internet access from Windows and also have proxy connections configured and after all, it is Windows 7.

That's pretty bad :) I'd be nice to have more info, and possibly whether it's new or not.

When I start Geany from within the MSYS2 environment, pasting some file works to some extend, at least no crash :). But the response look weird: […] The link below "\1" is actually also "\1".

The response_str in pastebin_parse_response is "https://www.geany.org/p/6vzVo" which is correct. So there seems to be a problem with parsing the URL. I don't know if this related to these changes or not.

This is weird, don't you have old leftover configuration for the plugin? This should only happen if your configuration has a [parse] section with an incorrect replace (if there are none, it would then use the default \1), e.g. one referencing a missing group in the search pattern.

@b4n b4n force-pushed the geniuspaste-libsoup3 branch from d336856 to 17af2ad Compare May 15, 2024 22:18
@b4n
Copy link
Member Author

b4n commented May 15, 2024

I rebased against master to have a fixed CI, plus added @eht16 Windows bundle fixes -- hopefully this will help testing and finding out what's wrong :)

@eht16
Copy link
Member

eht16 commented May 20, 2024

When trying to paste some file, Geany crashes for me when it is executed natively. I don't know yet why this happens and I didn't manage it yet to debug it with gdb. This might be related to my system where I use some firewall rules to block unwanted internet access from Windows and also have proxy connections configured and after all, it is Windows 7.

That's pretty bad :) I'd be nice to have more info, and possibly whether it's new or not.

I got it. After much of debugging and mostly fiddling around, I found two causes:

  • we need two more dependencies in addition to the ones above: "curl" and "zstd"
  • we need the GLib schemas for "gsettings-desktop-schemas"

The letter one is a bit tricky because some of those schemas are installed already in the Geany bundle and there we compile them also to share/glib-2.0/schemas/gschemas.compiled.
But here we are in G-P and it has its own bundle creation which doesn't know anything about we did for the Geany bundle. If we would compile the schemas in the G-P bundle, we would override whatever was in the Geany bundle.
So far, I see only bad options:

  • in the G-P bundle, we install GLib, GTK and maybe more dependencies to have all schemas we need and then compile them, on installation of the plugins the compiled schema bundle would override the one installed by Geany
  • in the Geany bundle, we install "gsettings-desktop-schemas" and let them compile as before. This is what I have tried and it works fine. But only as long as we have any dependency in G-P which brings more schemas and those won't get compiled.

This is probably such a case where a combined installer with everything included would be easier.

When I start Geany from within the MSYS2 environment, pasting some file works to some extend, at least no crash :). But the response look weird: […] The link below "\1" is actually also "\1".
The response_str in pastebin_parse_response is "https://www.geany.org/p/6vzVo" which is correct. So there seems to be a problem with parsing the URL. I don't know if this related to these changes or not.

This is weird, don't you have old leftover configuration for the plugin? This should only happen if your configuration has a [parse] section with an incorrect replace (if there are none, it would then use the default \1), e.g. one referencing a missing group in the search pattern.

No leftover configuration. I could reproduce this with pastebin.geany.org on Linux and Windows.
After removing the keys from the [parse] section in geniuspaste/data/pastebin.geany.org.conf so that the section remains empty in the config, it works.
I think this is not related to the changes here and probably was already broken before.

With this change and the additional dependencies mentioned above, I've got it finally working on Windows!!!
https://www.geany.org/p/mD5RN/ <- pasted from within Geany on Windows!

@b4n what do you think about the schemas compile problem?

@b4n
Copy link
Member Author

b4n commented May 20, 2024

No leftover configuration. I could reproduce this with pastebin.geany.org on Linux and Windows.
After removing the keys from the [parse] section in geniuspaste/data/pastebin.geany.org.conf so that the section remains empty in the config, it works.

What does the section contain?? because here (Debian) it works with the shipped configuration, even in a fresh separate install and configuration directory.

@b4n what do you think about the schemas compile problem?

Can't we "simply" run glib-compile-schemas during GP (and possibly Geany) installation? it's e.g. what Linux distros would do (e.g. Debian has a trigger or whatever for whenever you install a new schema in the system directory so the compiled schemas are rebuilt)

@eht16
Copy link
Member

eht16 commented May 20, 2024

No leftover configuration. I could reproduce this with pastebin.geany.org on Linux and Windows.
After removing the keys from the [parse] section in geniuspaste/data/pastebin.geany.org.conf so that the section remains empty in the config, it works.

What does the section contain??

What's in GIT master: https://github.com/geany/geany-plugins/blob/master/geniuspaste/data/pastebin.geany.org.conf#L12

because here (Debian) it works with the shipped configuration, even in a fresh separate install and configuration directory.

Weird, here is Debian Testing but Geany is installed from GIT master.

@b4n what do you think about the schemas compile problem?

Can't we "simply" run glib-compile-schemas during GP (and possibly Geany) installation? it's e.g. what Linux distros would do (e.g. Debian has a trigger or whatever for whenever you install a new schema in the system directory so the compiled schemas are rebuilt)

Hmm, this could work. I'll give it a try soon.

@b4n
Copy link
Member Author

b4n commented May 20, 2024

Weird, here is Debian Testing but Geany is installed from GIT master.

Yeah I meant separate Git install, not from Debian.

But I got it, it's likely just you using a newer GLib than I am, and they made a incompatible change: https://gitlab.gnome.org/GNOME/glib/-/issues/3098

@b4n
Copy link
Member Author

b4n commented May 20, 2024

@eht16 paste configuration issue should be fixed. But indeed it's unrelated to this PR, just to you using a newer GLib

@xiota
Copy link
Contributor

xiota commented May 20, 2024

What's the range of glib versions this plugin is supposed to work with?

@b4n
Copy link
Member Author

b4n commented May 21, 2024

@xiota I don't know exactly the minimum version, but practically any should be ok, why you ask?
The issue mentioned above is a change of behavior in glib 2.79. The plugin was admittedly using invalid syntax in the configuration files, but it went unnoticed because it worked fine.

@xiota
Copy link
Contributor

xiota commented May 21, 2024

I was curious about the upper limit because of problems described as being related to "newer GLib". But I guess the "invalid syntax" has been fixed. (I didn't notice earlier the commit changing backslashes.)

@eht16
Copy link
Member

eht16 commented May 25, 2024

@eht16 paste configuration issue should be fixed. But indeed it's unrelated to this PR, just to you using a newer GLib

Yes, it is fixed. Thanks for fixing it!

Can't we "simply" run glib-compile-schemas during GP (and possibly Geany) installation? it's e.g. what Linux distros would do (e.g. Debian has a trigger or whatever for whenever you install a new schema in the system directory so the compiled schemas are rebuilt)

Hmm, this could work. I'll give it a try soon.

Done in geany/geany#3885 and #1352.

Together with the bundle changes of this PR, this should work fine.

@eht16
Copy link
Member

eht16 commented Jun 15, 2024

@b4n do you mind rebasing on current master? Then we'll get easy to test installers from the CI and could finally merge this one.

@b4n b4n force-pushed the geniuspaste-libsoup3 branch from 0e2de9f to 9c0e16f Compare June 15, 2024 12:23
@b4n
Copy link
Member Author

b4n commented Jun 15, 2024

@eht16 sure, done.

@eht16
Copy link
Member

eht16 commented Jun 15, 2024

We are getting closer :). "gsettings-desktop-schemas" is missing in the dependency list in build/gtk-bundle-from-msys2.sh (sorry if I wasn't clear about it before).
This package contains the GLib schemas files necessary for the "glib-networking" package.

@b4n
Copy link
Member Author

b4n commented Jun 15, 2024

@eht16 done (in case you didn't see the commit)

@eht16
Copy link
Member

eht16 commented Jun 16, 2024

Seeen it but didn't have time until now.

With the new installers pasting to geany.org works fine now.

@b4n b4n force-pushed the geniuspaste-libsoup3 branch from 7a2fa7c to 4e93f0b Compare June 16, 2024 09:30
@b4n b4n merged commit bc2bf39 into geany:master Jun 16, 2024
2 checks passed
@b4n
Copy link
Member Author

b4n commented Jun 16, 2024

Merged, so we're one step closer to libsoup3 everywhere.

This doesn't mean we can't still drop the plugin if people want, I still don't mind even if I spent some time on this 🙂

@xiota
Copy link
Contributor

xiota commented Jun 16, 2024

This doesn't mean we can't still drop the plugin if people want, I still don't mind even if I spent some time on this 🙂

This type of plugin seems inherently dangerous... But up to people to install and run what they want.

@b4n
Copy link
Member Author

b4n commented Jun 16, 2024

@xiota see #1361 which might help

@b4n b4n added this to the 2.1.0 milestone Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants