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

XdpAppInfo refactoring, Part 2 #1369

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

swick
Copy link
Contributor

@swick swick commented Jun 5, 2024

This is a continuation of #1366.

This moves the app info kind specific code to subclasses, the pid mapping stuff to utils and reduces the scope of XdpAppInfo.

@swick swick force-pushed the wip/xdp-app-info-subclasses branch 3 times, most recently from 1c5df96 to aaf69ba Compare June 8, 2024 13:25
@swick swick marked this pull request as ready for review June 8, 2024 13:25
@swick swick force-pushed the wip/xdp-app-info-subclasses branch from aaf69ba to a034dff Compare June 11, 2024 16:59
swick added 25 commits June 18, 2024 17:48
They are only used internally and there is no reason to expose them in
the header.
This helps decoupling the util function from the XdpAppInfo.
Gets the value set by xdp_set_documents_mountpoint. This helps
decoupling the utils from the XdpAppInfo further.
This allows us to have a clean split between utils and the XdpAppInfo
which has become a complex free standing thing.
This moves around some functions so related code is close to each other
and also fixes the formatting of functions arguments.

There are no functional changes at all.
This deprecates impl.portal.Background.EnableAutostart. It turns out
portal backends implement this the same because there is no user
interaction and creating a file at the right place doesn't allow for
much divergence.

How the autostart file has to look however can depend on the kind of app
it is supposed to start. I.e. a flatpak app needs to change the Exec
line and wants to set X-Flatpak.
We encapsulate all the differences in XdpAppInfo now so there is no need
to expose what kind of app the instance is of.

We still have xdp_app_info_is_host to allow checking for a more
privileged process but we should try to get rid of it in the future as
well.
They are unused now that we have validate_autostart and
validate_dynamic_launcher.
We start using -private.h files to separate API from things we have to
expose for other reasons, like tests. This also starts marking symbols
with XDP_EXPORT and XDP_EXPORT_TEST which helps documenting their use.
This implements XdpAppInfo subclasses for host, tests, flatpak and snap.
They are unused for now and will get instantiated by a later commit.
They are initialized by xdp_app_info_initialize when the object gets
created and will stay valid until it is disposed.

A later commit will start using them to implement existing functions.
The pidns is derived from the pidfd of a process from within the same
pidns as the app.
Now that pid mapping is done by getting the pidns from the XdpAppInfo
and then using xdp-utils to do the actual mappings, we can remove the
mapping functionality from XdpAppInfo.
There is no remaining user of this function so we can make it private
and remove it from the header.
Implement existing functions using the new private fields of XdpAppInfo
and the new vfuncs of the class. Also instantiate the subclasses.

This allows us to remove the app info kind and the kind specific fields
from XdpAppInfo and delegates all of that to the subclasses.
@swick swick force-pushed the wip/xdp-app-info-subclasses branch from a034dff to 40f1779 Compare June 18, 2024 15:54
@swick
Copy link
Contributor Author

swick commented Jun 18, 2024

Rebased on top of the changes from Part 1.

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.

None yet

1 participant