Make the search order for portals.conf consistent with mimeapps.list #1082
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we would search for a user-specific portals.conf as highest precedence, and then iterate through desktop environments, searching for a user-specific desktop-specific file followed by a system-wide desktop-specific file. Now, the precedence order is consistently that directories are most important, we only pay attention to desktop environments within a single directory, and desktop-specific configuration is always higher-precedence than generic within the same directory. Previously, desktop environments and OS vendors had to install their per-desktop configuration into /etc, contradicting the general design principle of putting defaults in /usr and reserving /etc for sysadmin overrides. Now, we search both /etc and /usr/share (with typical environment variables and build-time settings). Previously, we didn't fully follow the XDG Base Directory specification when looking up per-user configuration; now we do. This is potentially significant for users who have more than one per-user directory, for example a machine-specific directory within a home directory shared between machines. Update documentation accordingly. Resolves: flatpak#1081 Signed-off-by: Simon McVittie <smcv@collabora.com>
smcv
commented
Aug 24, 2023
| const char **desktops; | ||
| const char *portal_dir; | ||
|
|
||
| desktops = get_current_lowercase_desktops (); | ||
|
|
||
| /* We need to override this in the tests */ | ||
| portal_dir = g_getenv ("XDG_DESKTOP_PORTAL_DIR"); |
There was a problem hiding this comment.
I left this as an intentionally undocumented implementation detail, since it's only there for the tests as far as I can see.
smcv
commented
Aug 24, 2023
Comment on lines
+495
to
+497
| /* ${datadir}/xdg-desktop-portal/(DESKTOP-)portals.conf */ | ||
| if (load_config_directory (DATADIR "/" XDP_SUBDIR, desktops, opt_verbose)) | ||
| return; |
There was a problem hiding this comment.
In practice this is redundant with XDG_DATA_DIRS but I thought we should probably search it anyway?
GeorgesStavracas
approved these changes
Aug 24, 2023
|
It doesn't need to be immediately, but I'd like to get a 1.17.1 out with this change at some point, so that when we start asking desktop environment maintainers (upstream or downstream) to install their own |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we would search for a user-specific portals.conf as highest precedence, and then iterate through desktop environments, searching for a user-specific desktop-specific file followed by a system-wide desktop-specific file. Now, the precedence order is consistently that directories are most important, we only pay attention to desktop environments within a single directory, and desktop-specific configuration is always higher-precedence than generic within the same directory.
Previously, desktop environments and OS vendors had to install their per-desktop configuration into /etc, contradicting the general design principle of putting defaults in /usr and reserving /etc for sysadmin overrides. Now, we search both /etc and /usr/share (with typical environment variables and build-time settings).
Previously, we didn't fully follow the XDG Base Directory specification when looking up per-user configuration; now we do. This is potentially significant for users who have more than one per-user directory, for example a machine-specific directory within a home directory shared between machines.
Update documentation accordingly.
Resolves: #1081