[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
This PR won (IMO), so it's the one to review @frlan -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2089096951 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
> hum… @elextr @xiota so what's your conclusion? Depend on libsoup3 as this PR > because it's HTTP, or depend on a GVFS backend for HTTP? Waiting for https://github.com/geany/geany-plugins/pull/1340#issuecomment-2079741003 If it works `?` #1340 `:` #1336 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2080317675 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
See #1340 if wanted. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2079550087 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Note that writing an async version of the feature, while reasonably easy, is a little bit more work. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2079476011 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
hum… @elextr @xiota so what's your conclusion? Depend on libsoup3 as this PR because it's HTTP, or depend on a GVFS backend for HTTP? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2079474204 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Yeah I guess using GIO directly for this kind of super simple GET would make sense Indeed. I also think that gvfs will indeed be part of all reasonable GIO installations, but I could be wrong. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2079187953 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Someone could run the test program on windows or mac to find out. I no longer have (easy) access to either. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078735830 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Well, I interpret the very first words in the readme "GVfs is a userspace virtual filesystem implementation for GIO" to mean its part of GIO. IIUC it provides the implementations for the abstract file/URL/dbus stream operations in GIO and thats why you get an error if its not installed and you try to read anything other than a local file. Never mind, its was worth a try, and thanks for writing that test program. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078728259 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
It is separate from GIO. This is the project page: https://gitlab.gnome.org/GNOME/gvfs Debian package, appears to be `gvfs-backends` -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078714133 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
> Looks like it depends on gvfs... After uninstalling... Well, IIUC GVFS is _part_ of GIO (not to be confused with GnomeVFS), so if its uninstalled its not surprising parts of GIO don't work. But that probably means reading URLs won't work on Windows, dunno about Macos, so back to the soup I guess :stuck_out_tongue_winking_eye: -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078709049 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
I use whatever is provided by the package manager. Even on Debian, pkg-config is a dummy package that redirects to pkgconf. Their stance doesn't seem particularly "aggressive" in context because they call out "passive-aggressive people who try to argue with us". Seems they probably had some people who opened numerous issues reporting closely related problems that they were told multiple times are not bugs. It would be like... what if someone opened multiple issues for Geany complaining that it doesn't support various Qt6 features despite being told multiple times that they aren't bugs? Eventually, those complaints could be considered intentionally disruptive, and people who "insist" on continuing need to be cut off. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078702112 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Yeah, `pkg-config` is the Freedesktop original, `pkgconf` would appear to be a re-implementation. Your distro installs `pkgconf` and links the original command to it so existing scripts will work, if they happen to conform to what `pkgconf` considers "correct". If an existing script fails because it depends on some edge case in `pkg-config` tough. And very aggressive about it they are, "if someone insists on fixing such a non-bug, this constitutes a violation of our [Code of Conduct](https://gitea.treehouse.systems/ariadne/pkgconf/src/branch/master/CODE_OF_CONDUCT.md), which may be addressed by banning from this repository". -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078675523 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
> Unfortunately the GIO docs don't say when `g_file_new_from_URI()` and the > associated stream was supported (that I could see) but it works on the > machine I'm on ATM which is pretty olde. Looks like it depends on `gvfs`... After uninstalling... ``` $ ./test Error reading uri. ``` > PS I expect not everyone has your alias pkgconf == pkg-config On my computer, `pkg-config` is the symlink. Looks like [pkgconf](https://gitea.treehouse.systems/ariadne/pkgconf) and [pkg-config](https://www.freedesktop.org/wiki/Software/pkg-config/) are different projects. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078657909 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Unfortunately the GIO docs don't say when `g_file_new_from_URI()` and the associated stream was supported (that I could see) but it works on the machine I'm on ATM which is pretty olde. PS I expect not everyone has your alias pkgconf == pkg-config :wink: -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078637852 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
Size check is needed if this route is taken for the plugin... That code was intended to check whether `https://` is supported because I saw only `file://` mentioned in the docs. Typical version string is <8 bytes. Even with words like `alpha` or `pre-release`, I figured 32 bytes is enough for the test. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078600197 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
@xiota neat, and simple, although it has one theoretical bug (though unlikely in practice) [hint: what if 32 bytes were returned]. Just for general information, Geany used (a __long__ time ago) to avoid GIO, which is probably why the plugin used libsoup directly. But now GIO is required by Geany so its available to the plugin. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078565593 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
> `g_file_new_for_uri()` and `g_file_read()` (or
> `g_file_read_async()/g_file_read_finish()`
It works...
```C
#include
#include
int main() {
GFile *file = g_file_new_for_uri("https://geany.org/service/version/;);
GError *error = NULL;
GFileInputStream *stream = g_file_read(file, NULL, );
if (!error) {
printf("No error.\n");
guint8 *buffer = NULL;
gsize size = 0;
buffer = (guint8 *)g_malloc(32);
gboolean success = g_input_stream_read_all((GInputStream *)stream, buffer,
32, , NULL, );
if (success) {
buffer[size] = '\0';
printf("Data: %s\n", buffer);
} else {
printf("Error reading stream.\n");
}
} else {
printf("Error reading uri.\n");
}
return 0;
}
```
```bash
$ gcc test.c -o test $(pkgconf --libs gio-2.0) $(pkgconf --cflags gio-2.0)
$ ./test
No error.
Data: 2.0.0
```
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078546850
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
I don't know enough about libsoup to sensibly review (and don't have the time to learn), but since Geany uses GIO why not `g_file_new_for_uri()` and `g_file_read()` (or `g_file_read_async()/g_file_read_finish()` to not block the UI) instead of using libsoup directly? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1336#issuecomment-2078454585 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
@b4n commented on this pull request.
> @@ -185,15 +191,30 @@ version_compare(const gchar *current_version)
}
-static void update_check_result_cb(SoupSession *session,
-SoupMessage *msg, gpointer user_data)
+static gchar *bytes_to_string(GBytes *bytes)
+{
+gsize bytes_size = g_bytes_get_size(bytes);
+gchar *str = g_malloc(bytes_size + 1);
+memcpy(str, g_bytes_get_data(bytes, NULL), bytes_size);
Yeah that's exactly the reason: the bytes have no reason to be NUL-terminated,
as it's from reading the raw data from the webserver (and does not explicitly
add a NUL itself from what I can gather). So this helper is to make it a C
string, not really caring if there's a NUL earlier because a valid response
won't have one anyway.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1336#discussion_r1579201225
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
@xiota commented on this pull request.
> @@ -185,15 +191,30 @@ version_compare(const gchar *current_version)
}
-static void update_check_result_cb(SoupSession *session,
-SoupMessage *msg, gpointer user_data)
+static gchar *bytes_to_string(GBytes *bytes)
+{
+gsize bytes_size = g_bytes_get_size(bytes);
+gchar *str = g_malloc(bytes_size + 1);
+memcpy(str, g_bytes_get_data(bytes, NULL), bytes_size);
I see... thanks.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1336#discussion_r1578857604
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
@elextr commented on this pull request.
> @@ -185,15 +191,30 @@ version_compare(const gchar *current_version)
}
-static void update_check_result_cb(SoupSession *session,
-SoupMessage *msg, gpointer user_data)
+static gchar *bytes_to_string(GBytes *bytes)
+{
+gsize bytes_size = g_bytes_get_size(bytes);
+gchar *str = g_malloc(bytes_size + 1);
+memcpy(str, g_bytes_get_data(bytes, NULL), bytes_size);
Perhaps the response body might not be a nul terminated string, so its copied
to one.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1336#discussion_r1578813084
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] updatechecker: Port to libsoup3 and small other fixes (PR #1336)
@xiota commented on this pull request.
> @@ -185,15 +191,30 @@ version_compare(const gchar *current_version)
}
-static void update_check_result_cb(SoupSession *session,
-SoupMessage *msg, gpointer user_data)
+static gchar *bytes_to_string(GBytes *bytes)
+{
+gsize bytes_size = g_bytes_get_size(bytes);
+gchar *str = g_malloc(bytes_size + 1);
+memcpy(str, g_bytes_get_data(bytes, NULL), bytes_size);
I don't know any version of libsoup, but wondering why wrap
`g_bytes_get_data()` with another function and memcopy that has to be freed
later, rather than `g_bytes_get_data()` directly, and letting `g_bytes_unref()`
take care of it.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1336#pullrequestreview-2021421076
You are receiving this because you are subscribed to this thread.
Message ID:
