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.

Sign up for GitHub

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

Jump to bottom

Added support for 'mapping' Snaps #882

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

Added support for 'mapping' Snaps #882

wants to merge 1 commit into from

Conversation

ashuntu
Copy link

@ashuntu ashuntu commented Sep 13, 2022

Allows Snaps to have their pids "mapped" if each pid app info matches, namely for use cases such as this.

@ashuntu ashuntu mentioned this pull request Sep 13, 2022
Merged
@sergio-costas
Copy link

Looks good to me.

sergio-costas
sergio-costas approved these changes Sep 15, 2022
View reviewed changes
Copy link

@sergio-costas sergio-costas left a comment

Choose a reason for hiding this comment

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

LGTM

3v1n0
3v1n0 suggested changes Nov 1, 2022
View reviewed changes
src/xdp-utils.c Outdated Show resolved Hide resolved
src/xdp-utils.c Outdated Show resolved Hide resolved
3v1n0
3v1n0 suggested changes Nov 17, 2022
View reviewed changes
src/xdp-utils.c Outdated Show resolved Hide resolved
src/xdp-utils.c Outdated Show resolved Hide resolved
3v1n0
3v1n0 reviewed Nov 24, 2022
View reviewed changes
src/xdp-utils.c Outdated Show resolved Hide resolved
3v1n0
3v1n0 reviewed Nov 24, 2022
View reviewed changes
Copy link
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

Looks good, a part few minor details.

src/xdp-utils.c Outdated Show resolved Hide resolved
@3v1n0
Copy link
Contributor

3v1n0 commented Nov 24, 2022

Once you've applied the suggestions, please, squash all the commits and write a commit explaining the rationale for this clearer.

@ashuntu ashuntu requested a review from 3v1n0 August 29, 2023 18:59
3v1n0
3v1n0 reviewed Aug 30, 2023
View reviewed changes
Copy link
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

Code looks good now, see if we prefer having a better error code.

As per the rest, please squash the two commits together and provide a clear commit message (and PR descriptions) so that also who is less familiar with the change could understand it.

See: https://wiki.gnome.org/Git/CommitMessages

src/xdp-utils.c Outdated Show resolved Hide resolved
@ashuntu
Add support for Snaps to use portals
08007ff 
Validates that pids are owned by Snaps, allowing them to be mapped and passed through
to a portal interface; Snaps were previously denied pid mapping.

Initially created to allow GameMode to use the portal dbus from within a snap:
FeralInteractive/gamemode#385
This was referenced Oct 12, 2023
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@3v1n0 3v1n0 3v1n0 requested changes

@kenvandine kenvandine kenvandine left review comments

@sergio-costas sergio-costas sergio-costas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Triage
Status: Needs Triage
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashuntu @sergio-costas @3v1n0 @kenvandine