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

portal-impl: fix config ordering (supersedes #1240) #1378

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SoumyaRanjanPatnaik
Copy link
Contributor

@SoumyaRanjanPatnaik SoumyaRanjanPatnaik commented Jun 17, 2024

This PR aims to supersede #1240.

Original PR message:

Quoting portals.conf(5):

Each key in the group contains a semi-colon separated list of portal backend
implementation, to be searched for an implementation of the requested interface,
in the same order as specified in the configuration file.

But this wasn't actually true. If the portals were set to z;y and z and y both implemented the same portal, y would be used.

Fixing this requires reworking how portals are selected — going through the config file and selecting the first available configured portal, rather than going through each known portal and checking whether it's allowed in the config file.

find_all_portal_implementations() is unchanged. The portal-first approach is fine for it, and it would be difficult for it to share as much code with find_portal_implementation() now that it's written to stop as soon as a configured portal is found.

Changes from the original PR include:

  1. Rebased and resolved conflicts from main.
  2. Cleanup redundant code for handling none case since find_portal_implementation returns early if the portal interface prefers none.
  3. Add check to find_portal_implementation to ensure that the portal implementation selected from the config actually supports the interface.
  4. Created a seperate function find_default_implementation_iface to handle the default case, since find_portal_implementation only has access to dbus_name. The dbus_name for config->default_portal is always set to default, which will never match the name of the interface we are finding the implementation for.
  5. The original PR moved the code for GTK fallback into a seperate function that was in certain conditions invoked before the default / UseIn fallbacks were tried. Moved this code back into find_portal_implementation.
  6. Use the same logic for selecting portals in find_all_portal_implementations as well to ensure consistency.
  7. Cleanup unused functions.

Let me know if any changes are required or if you need to see a diff between the two PRs. I'm

Fixes: #1111

Quoting portals.conf(5):

> Each key in the group contains a semi-colon separated list of portal
backend
> implementation, to be searched for an implementation of the requested
interface,
> in the same order as specified in the configuration file.

But this wasn't actually true.  If the portals were set to z;y and z
and y both implemented the same portal, y would be used.

Fixing this requires reworking how portals are selected — going
through the config file and selecting the first available configured
portal, rather than going through each known portal and checking
whether it's allowed in the config file.

Fixes: flatpak#1111

Co-authored-by: Alyssa Ross <hi@alyssa.is>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

The portal config file does not actually specify the order of portals to be tried
1 participant