snap: add a way to add common slots to the OS snap #803

Merged
merged 4 commits into from Apr 7, 2016

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Apr 6, 2016

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

snap: add a way to add common slots to the OS snap
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
snap/special.go
+ "timeserver-control",
+ "timezone-control",
+ // XXX: those two should perhaps not be added by default
+ "unit7",
@jdstrand

jdstrand Apr 6, 2016

Contributor

typo

@zyga

zyga Apr 6, 2016

Contributor

Fixed, thanks.

snap/special.go
+
+// AddCommonSlotsToOSSnap adds slots of well-known interfaces to the OS snap.
+//
+// This function is indented to be used temporarily, before the OS snap is
@jdstrand

jdstrand Apr 6, 2016

Contributor

intended*

@zyga

zyga Apr 6, 2016

Contributor

Fixed. I wonder how I managed to confuse the two.

zyga added some commits Apr 6, 2016

snap: fix typo
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
snap: fix typo
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
snap/special.go
+ "system-observe",
+ "timeserver-control",
+ "timezone-control",
+ // XXX: those two should perhaps not be added by default
@jdstrand

jdstrand Apr 6, 2016

Contributor

They should not be added on an IoT device but should for snappy dimension on classic. I'm not sure of the timing of things, but snappy on classic will continue to be blocked unless these are added in some way (doesn't have to be here, but if not here, via some other declaration). One way to do this is a cheap check like the launcher does and see if /var/lib/dpkg/status exists. Since this is only a temporary check, perhaps a cheap check like this is ok.

@zyga

zyga Apr 7, 2016

Contributor

I'll add a card for that and do that after 16.04, for now this just gets us going and if we release the os snap again with properly defined slots we can drop this feature altogether.

snap/special.go
+//
+// It is assumed that slots have names matching the interface name. Existing
+// slots are not changed, only missing slots are added.
+func AddCommonSlotsToOSSnap(snapInfo *Info) error {
@niemeyer

niemeyer Apr 7, 2016

Contributor

Let's please name this AddImplicitSlots, and below handle only if Type == TypeOS, ignoring otherwise rather than erroring. It's okay that we don't have something for the other types right now.

That also means we can drop the error result.

@zyga

zyga Apr 7, 2016

Contributor

Done

Contributor

niemeyer commented Apr 7, 2016

One comment and LGTM

@niemeyer niemeyer added the Reviewed label Apr 7, 2016

snap: tweak AddImplicitSlots
This patch renames the function to AddImplicitSlots, removes the error
condition by simply ignoring other snap types and tweaks documentation
and tests to match. This way the function is always safe to load on any
snap and it might do useful things for other snaps in the future.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit e41a03a into snapcore:master Apr 7, 2016

4 checks passed

Integration tests Success
Details
autopkgtest Success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.4%) to 76.411%
Details

@zyga zyga deleted the zyga:snap-os-snap-slots branch Dec 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment