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

Use GUri instead of libsoup-2.4 and use webkit2gtk-4.1 #745

Closed
wants to merge 2 commits into from

Conversation

oreo639
Copy link
Contributor

@oreo639 oreo639 commented Mar 1, 2024

This updates the URI parsing for astroid --mailto to use Glib instead of libsoup-2.0 (since the api was removed in libsoup-3.0).

https://libsoup.org/libsoup-3.0/migrating-from-libsoup-2.html
#744

I tested the mailto parsing appears to work but please double check.

@gauteh
Copy link
Member

gauteh commented Mar 2, 2024

Does this WebKit version work?

@oreo639
Copy link
Contributor Author

oreo639 commented Mar 2, 2024

Does this WebKit version work?

As far as I can tell, but please double check.
webkit2gtk-4.1 and webkit2gtk-4.0 are supposed to be, functionality wise and API wise, identical. (you have to explicitly opt in to -4.1 since it uses libsoup3 which cannot be loaded in to a process alongside libsoup2)

@oreo639
Copy link
Contributor Author

oreo639 commented Apr 20, 2024

Would you prefer for both gui and soup2 to be supported? (e.g. --with-soup2)
I did it this way to avoid ifdefs, but if you are still targeting Bionic I can add that. Or would you prefer the CI be updated to target newer releases?

(e.g. if --with-soup2 is specified, it will use soup-2.0 and disable the ability to use webkit2gtk-4.1 and without it, it will use guri and support both webkit2gtk-4.1 and webkit2gtk-4.0)

@gauteh
Copy link
Member

gauteh commented Apr 20, 2024 via email

@oreo639 oreo639 changed the title Use GUri instead of libsoup-2.0 and use webkit2gtk-4.1 Use GUri instead of libsoup-2.4 and use webkit2gtk-4.1 Apr 22, 2024
@jorsn
Copy link
Member

jorsn commented Jun 3, 2024

This looks good, it works for me, with both webkit2gtk-4.1 and webkit2gtk-4.0.
It is compatible with #734 and #747.

@jorsn
Copy link
Member

jorsn commented Jun 4, 2024

Unfortunately, the CI is not active, here. I pushed it in my repo with some CI updates, and saw that it also breaks Debian bullseye (glib 2.66) and Ubuntu focal (2.64) (for versions see repology.

Should we support bullseye and/or focal?
Both are still supported, but both glib versions are old and almost all distros seem to have glib >= 2.68.

@gauteh
Copy link
Member

gauteh commented Jun 4, 2024

I recommend moving to Ubuntu and only supporting recent distros, we don't want to spend the effort we have available on oldish libraries.

@ibuclaw
Copy link
Contributor

ibuclaw commented Jun 4, 2024

I recommend moving to Ubuntu and only supporting recent distros, we don't want to spend the effort we have available on oldish libraries.

Agreed, EOL distributions are of no interest to anyone.

@ibuclaw
Copy link
Contributor

ibuclaw commented Jun 4, 2024

This looks to be identical to what I did as well to fix Astroid for webkit2gtk-4.1 (cabdf83), so this gets a 👍 from me.

@jorsn
Copy link
Member

jorsn commented Jun 4, 2024

I submitted the competing #748. I'd be happy if you voice your opinions about it, compared to this one.

@oreo639
Copy link
Contributor Author

oreo639 commented Jun 4, 2024

Bullseye test fails because of missing G_URI_FLAGS_SCHEME_NORMALIZE, I added it because it is listed with the libsoup compatibility defines: https://gitlab.gnome.org/GNOME/libsoup/-/blob/a94a3a9a6ba14d6873c70f9931fe2d001442b14c/libsoup/soup-uri-utils.h#L40

However, for mailto links it should be useless as it only operates on these schemes: https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/guri.c#L760 https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/guri.c#L804
(e.g. it ensures that http://example.org becomes http://example.org:80/)

So I'll remove it.

@jorsn
Copy link
Member

jorsn commented Jun 4, 2024

When you change the PR, maybe you can also add the removal of ubuntu focal, since accepting this PR means dropping focal (which still has official support) from the CI.

4 years old release and doesn't support glib 2.66+ (necessary for GUri).
@jorsn
Copy link
Member

jorsn commented Jun 18, 2024

I merged my PR #748 now instead of this. Thank you for this, still! #748 was born out of a closer look at this PR, here, and noticing that there was mailto handling somewhere else that did not use soup/guri. So, I unified them, which solved also #744. I might not have noticed this without this PR.

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.

None yet

4 participants