-
Notifications
You must be signed in to change notification settings - Fork 58
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
snapcraft.yaml: improve version,base and plugs #161
base: master
Are you sure you want to change the base?
Conversation
This results in a version number like: 2023.02.07.g0746598 which gives more information than the curent hardcoded version.
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.
very nice!
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 but the personal-files plug names need to be changed to match the usual conventions.
…llow personal-files-interface convention
dot-spread: | ||
interface: personal-files | ||
read: | ||
- $HOME/.spread |
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.
Some concerns during a discussion: as $HOME inside the confinement does not point to the real /home but ~/snap/sparead/ this seems a bit useless without further changes. So this needs some more investigation/thinking.
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 whilst $HOME
in the snap environment points to $SNAP_USER_DATA
, in this case $HOME
gets used as an AppArmor rule and will translate to @{HOME}
- see formatPath()
in interfaces/builtin/common_files.go
- https://github.com/snapcore/snapd/blob/master/interfaces/builtin/common_files.go#L70 - so this works as intended.
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 believe that what @mvo5 is alluding to is that perhaps spread needs to be aware of this detail
This PR adds three patches, I'm happy to propose individual PRs instead if that is preferred:
I'm happy to tweak things as needed, especially the version number seems to be something that people have different opinions about.