-
Notifications
You must be signed in to change notification settings - Fork 583
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
many: extract desktop-ids helpers from wrappers package into snap package #14456
many: extract desktop-ids helpers from wrappers package into snap package #14456
Conversation
…kage Those helpers are extracted to be reused when generating apparmor rules specific to desktop-file-ids Signed-off-by: Zeyad Gouda <[email protected]>
// MangleDesktopFileName returns the file name prefixed with Info.DesktopPrefix() | ||
// unless its name (without the .desktop extension) is listed under the | ||
// desktop-file-ids desktop interface attribute. | ||
func (s *Info) MangleDesktopFileName(desktopFile string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to be exported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was trying to have the mangling consistent between the desktop wrappers and apparmor generation to avoid any drifts in implementation between the two.
Desktop wrappers, need to open the snap's desktop files under <snap-mount>/meta/gui
, do some processing and write them (with the mangled names) under SnapDesktopFilesDir
. deriveDesktopFilesContent
Apparmor rules generation (#14444 which builds on top of this PR) only needs the installed mangled names but since generation profiles happens after mounting but before installing the desktop files, we need to infer them using DesktopFilesFromMount
with MangleFileNames=true
. getDesktopFileRules
I would appreciate any suggestions to make it simpler.
} | ||
|
||
for i, df := range desktopFiles { | ||
desktopFiles[i], err = s.MangleDesktopFileName(df) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps if do not export this function then it could take a list of declared desktop plug IDs? otherwise it'll end up calling the same DesktopFileIDs() for every desktop file we find
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I was hesitant about this, having it more readable vs more efficient, I went with the readable approach since it was exported.
Open to any suggestions on simplifying this while keeping the implementation consistent between the desktop wrappers and apparmor rules generation #14456 (comment)
Signed-off-by: Zeyad Gouda <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Those helpers are extracted to be reused in a later PR when generating apparmor rules specific to desktop-file-ids.
Those helpers are needed by #14444.