Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
dirs: use the right snap mount dir for the distribution #2845
Conversation
zyga
added some commits
Feb 14, 2017
zyga
added this to the
2.23 milestone
Feb 14, 2017
| + DistroLibExecDir = filepath.Join(rootdir, "/usr/lib/snapd") | ||
| + } | ||
| + | ||
| + CoreLibExecDir = filepath.Join(rootdir, "/usr/lib/snapd") |
Conan-Kudo
Feb 14, 2017
Contributor
For now, I suppose this is fine, since we don't have support for different bases and core snaps yet, but just a fair warning that this assumption is not likely to stick around either...
Conan-Kudo
Feb 14, 2017
Contributor
It occurs to me that you could test for the existence of /usr/libexec/snapd, and if it exists, use it, otherwise just fall back to /usr/lib/snapd. That's much simpler than trying to use crazy case statements to switch things around.
zyga
Feb 14, 2017
Contributor
For core that's a good assumption. For base that won't be a factor (I think).
As for the second advice I agree, I will look at making all the runtime checks comprehensive and not based on the os-release but rather on the important property at hand, I just wanted to land this quickly as it bears a lot of fruit.
Conan-Kudo
Feb 14, 2017
Contributor
@zyga Like I said, for now the core snap libexecdir thing is fine, but once it becomes possible for me to make a Fedora Snappy Core using a core snap based from the build artifacts from Fedora infrastructure, that will not be valid anymore.
If the change to support this difference is trivial, I'd prefer to see it handled now rather than later, since you're already messing with this code.
zyga
Feb 14, 2017
Contributor
We will not do fedora snappy core, we will do a fedora base, the core will stay unique but it will not be used as the root for snap execution anymore. I think we are in agreement, it was just the naming that got confusing.
Conan-Kudo
approved these changes
Feb 14, 2017
Anyway, aside from my comments above, it looks fine to me.
Conan-Kudo
referenced this pull request
Feb 14, 2017
Merged
many: differentiate between "distro" and "core" libexecdir #2844
|
@zyga Can you rebase this PR against master? |
zyga commentedFeb 14, 2017
Stacked on #2844
Some distributions use /var/lib/snapd/snap as the place where snaps are mounted
so make this a dynamic runtime choice made on startup of snapd and other tools.
Signed-off-by: Zygmunt Krynicki me@zygoon.pl