dirs,interfaces,overlord,snap,snapenv,test: export per-snap XDG_RUNTIME_DIR per user (LP: #1620442) #2281
Conversation
| @@ -475,6 +475,7 @@ func (s *infoSuite) TestDirAndFileMethods(c *C) { | |||
| c.Check(info.UserDataDir("/home/bob"), Equals, "/home/bob/snap/name/1") | |||
| c.Check(info.UserCommonDataDir("/home/bob"), Equals, "/home/bob/snap/name/common") | |||
| c.Check(info.CommonDataDir(), Equals, "/var/snap/name/common") | |||
| c.Check(info.UserXdgRuntimeDir(12345), Equals, "/run/user/12345/snap.name") | |||
There was a problem hiding this comment.
Looks like we want to test XdgRuntimeDir here has well.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
| @@ -319,6 +319,10 @@ var defaultTemplate = []byte(` | |||
| # access in /dev/shm for shm_open() and files in subdirectories for open() | |||
| /{dev,run}/shm/snap.@{SNAP_NAME}.** mrwlkix, | |||
|
|
|||
| # Snap-specific XDG_RUNTIME_DIR that is based on the UID of the user | |||
There was a problem hiding this comment.
Can we use + rather than *, just to ensure that component exists please.
There was a problem hiding this comment.
@zyga - I don't understand this comment: "Can we use + rather than *, just to ensure that component exists please." in the context of where you put the comment. Can you elaborate? (the apparmor rule is correct)
| @@ -56,6 +56,9 @@ type PlaceInfo interface { | |||
|
|
|||
| // CommonDataHomeDir returns the per user data directory common across revisions of the snap. | |||
| CommonDataHomeDir() string | |||
|
|
|||
| // XdgRuntimeDir returns the XDG_RUNTIME_DIR directories of the snap. | |||
There was a problem hiding this comment.
One directory or many directories? Code wise it looks like just one. If someone please adjust the comment.
There was a problem hiding this comment.
Ah if this is supposed to be a glob it should be more explicit.
There was a problem hiding this comment.
@zyga - this is one directory per user, but cleanup will be a glob since multiple users might have used the snap and have the directory. Ie:
- I launch a snap 'foo' and need XDG_RUNTIME_DIR=/run/user/1000/snap.foo (ie UserXdgRuntimeDir())
- you launch a snap 'foo' and need XDG_RUNTIME_DIR=/run/user/1001/snap.foo (ie UserXdgRuntimeDir())
- the sysadmin does 'snap remove foo' (ie XdgRuntimeDir() glob for /run/user/*/snap.foo
I'll improve the comments and function names.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
…into per-snap-xdg-runtime-dir
|
It looks like the XDG_SESSION_ID check that @zyga added is causing the autopkgtests to fail. This variable wasn't added as part of this commit so removing from the spread test. |
|
Looks good, please feel free to merge once the travis tests are green. The autopkgtests have some known issue (after merging master it should be much better) so its ok to ignore them for now. |
In support of Ubuntu Personal and xdg-aware snaps, this PR exports XDG_RUNTIME_DIR as a snap and user-specific directory. AppArmor rules are added to allow creating and using the directory. Note: the location of this directory follows the current convention of /run/user/$UID except that it creates a snap-specific subdirectory under it which follows the typical snappy file naming conventions. Eg, for a snap named 'foo', XDG_RUNTIME_DIR=/run/user/$UID/snap.foo. $UID is set to os.Geteuid() to ensure the proper uid is used when invoked with sudo.
A separate PR for snap-confine will create this directory on behalf of the user. These PR can land independently of each other.