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

Support XDP Desktop Portal proxy resolver #295

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Support XDP Desktop Portal proxy resolver #295

merged 1 commit into from
Apr 16, 2024

Conversation

janbrummer
Copy link
Contributor

In order to make use of libproxy in a flatpak environment, we need to support XDP proxy resolver. Add support.

@DimStar77 Could you please have a look and review this one?

@janbrummer
Copy link
Contributor Author

Check possibility of adding a flatpak test

@janbrummer
Copy link
Contributor Author

https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/blob/master/elements/components/libproxy.bst?ref_type=heads

needs to be changed to enable curl and duktape for automatic proxy support.

@janbrummer janbrummer force-pushed the xdp-config branch 2 times, most recently from 825f87b to edd41ba Compare April 10, 2024 19:38
@janbrummer
Copy link
Contributor Author

@mcatanzaro In case you have some time, could you please have a look at this PR? Works fine in my test flatpak app, but maybe i missed something important....

Next step would be to adjust freedesktop runtime

@janbrummer
Copy link
Contributor Author

https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/blob/master/elements/components/libproxy.bst?ref_type=heads

needs to be changed to enable curl and duktape for automatic proxy support.

Recommended options:


                "-Dconfig-env=false",
                "-Dconfig-kde=false",
                "-Dconfig-gnome=false",
                "-Dconfig-sysconfig=false",
                "-Dconfig-xdp=true",
                "-Dintrospection=false",
                "-Drelease=true",
                "-Ddocs=false"

@mcatanzaro
Copy link
Contributor

The freedesktop runtime is not only used by flatpaks. It's also used by host systems, like GNOME OS, and also for images for embedded systems. So a "disable everything" approach doesn't make sense. We should decide which backend to use at runtime, not compile time.

meson_options.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@mcatanzaro mcatanzaro left a comment

Choose a reason for hiding this comment

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

I think you should enable it by default.

@mcatanzaro
Copy link
Contributor

Hm, thorny question: what priority should it have, relative to the other settings providers? If this is not the highest-priority provider, then the other providers will get used instead and possibly return bad results because libproxy doesn't realize it's not running on the host system, right? But if this is the highest-priority provider, then all the other non-Windows providers are suddenly no longer getting tested anymore, because almost everyone will have xdg-desktop-portal installed on Linux. And libproxy will probably be slower, too, since it has to rely on sync D-Bus calls.

Hm, another possible problem: the proxy resolver portal could itself be using libproxy, and in practice it will whenever running outside GNOME. So if all backends are always enabled at build time, then this will turn into an infinite loop where libproxy and the portal both try to look up proxy settings from each other. OK, proposed solution: libproxy should check if it's running under flatpak or snap, and only use the portal if so. That should work, right?

@janbrummer
Copy link
Contributor Author

janbrummer commented Apr 11, 2024

Hm, thorny question: what priority should it have, relative to the other settings providers? If this is not the highest-priority provider, then the other providers will get used instead and possibly return bad results because libproxy doesn't realize it's not running on the host system, right? But if this is the highest-priority provider, then all the other non-Windows providers are suddenly no longer getting tested anymore, because almost everyone will have xdg-desktop-portal installed on Linux. And libproxy will probably be slower, too, since it has to rely on sync D-Bus calls.

Hm, another possible problem: the proxy resolver portal could itself be using libproxy, and in practice it will whenever running outside GNOME. So if all backends are always enabled at build time, then this will turn into an infinite loop where libproxy and the portal both try to look up proxy settings from each other. OK, proposed solution: libproxy should check if it's running under flatpak or snap, and only use the portal if so. That should work, right?

At the moment every plugin is checked, but config-env does have an higher priority. In case no configuration is found, the plugin returns nothing at all. Only iff all plugins returned nothing, px manager will add a direct rule as a global return value.

The config-xdp plugin already checks whether it is running in a flatpak environment and disables itself in a non-flatpak environment. All the other plugins are still working and that might also be legit as flatpak permission can transfer e.g. access to sysconfig or dconf or kde configuration files..

So that means that the order in a flatpak environment would be the same as in a non flatpak environemnt:

  • config-env
  • all the rest

Sounds good to me, right?

@janbrummer
Copy link
Contributor Author

Recommended options:

                "-Dintrospection=false",
                "-Drelease=true",
                "-Ddocs=false"

@mcatanzaro
Copy link
Contributor

Yeah OK, sounds good, no problem then. Except you'll want to use the portal under snap too, not just flatpak.

In order to make use of libproxy in a flatpak environment, we need to
support XDP proxy resolver. Add support.
@janbrummer
Copy link
Contributor Author

Yeah OK, sounds good, no problem then. Except you'll want to use the portal under snap too, not just flatpak.

Thanks, I've added a snap check as well.

@janbrummer janbrummer merged commit 999ebe1 into main Apr 16, 2024
12 checks passed
@janbrummer janbrummer deleted the xdp-config branch April 25, 2024 18:17
@janbrummer
Copy link
Contributor Author

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

3 participants