Merge lp:~sergiusens/snappy/rollYourOwnCorePersonal into lp:~snappy-dev/snappy/snappy-moved-to-github

[Sergio Schvezov]

Snappy to expose the core package as personal or core depending on the release

This will affect snappy config (and maybe oem packages that define configs) so releases for snaps may have to be split up after this

[Michael Vogt (mvo)]

Thanks, this looks good! I have some question inline, probably mostly my own ignorance.

review: Needs Information

[Sergio Schvezov (sergiusens)]

On Mon, Jul 13, 2015 at 07:04:25AM -0000, Michael Vogt wrote:
> Review: Needs Information
>
> Thanks, this looks good! I have some question inline, probably mostly my own ignorance.
>
> Diff comments:
>
> > === modified file 'cmd/snappy/cmd_info.go'
> > --- cmd/snappy/cmd_info.go 2015-07-01 14:56:49 +0000
> > +++ cmd/snappy/cmd_info.go 2015-07-10 11:47:46 +0000
> > @@ -90,22 +91,12 @@
> > return nil
> > }
> >
> > -func ubuntuCoreChannel() string {
> > - parts, err := snappy.ActiveSnapsByType(pkg.TypeCore)
> > - if len(parts) == 1 && err == nil {
> > - return parts[0].Channel()
> > - }
> > -
> > - return "unknown"
> > -}
> > -
> > func info() error {
> > - release := ubuntuCoreChannel()
> > frameworks, _ := snappy.ActiveSnapNamesByType(pkg.TypeFramework)
> > apps, _ := snappy.ActiveSnapNamesByType(pkg.TypeApp)
> >
> > // TRANSLATORS: the %s release string
> > - fmt.Printf(i18n.G("release: %s\n"), release)
> > + fmt.Printf(i18n.G("release: %s\n"), release.String())
>
> This is nice (i.e. that all the code above gets killed :)
>

\o/

> > // TRANSLATORS: the %s an architecture string
> > fmt.Printf(i18n.G("architecture: %s\n"), snappy.Architecture())
> > // TRANSLATORS: the %s is a comma separated list of framework names
> >
> > === modified file 'release/release.go'
> > --- release/release.go 2015-05-15 13:33:27 +0000
> > +++ release/release.go 2015-07-10 11:47:46 +0000
> > @@ -51,9 +51,18 @@
> > return rel.String()
> > }
> >
> > +// Flavor returns the flavor for the release.
> > +func Flavor() string {
> > + return rel.flavor
> > +}
> > +
> > // Override sets up the release using a Release
> > -func Override(r Release) {
> > - rel = r
> > +func Override(flavor, series, channel string) {
>
> What the purpose of this function? AFAICT its only used in systemimage_test.go. Or is it there to allow overrides in the future? If thats the case, why will we need "overrides"? It seems like the channel.ini gives us the "ubuntu-{core,personal}" information as part of its Setup (or am I missing something)?
>

You forget about u-d-f and obtaining the first oem snap that has all the
info we need ;-)

> > + rel = Release{
> > + flavor: flavor,
> > + series: series,
> > + channel: channel,
> > + }
> > }
> >
> > // Setup is used to initialiaze the release information for the system
> >
> > === modified file 'snappy/systemimage.go'
> > --- snappy/systemimage.go 2015-07-01 14:48:33 +0000
> > +++ snappy/systemimage.go 2015-07-10 11:47:46 +0000
> > @@ -97,7 +98,7 @@
> >
> > // Name returns the name
> > func (s *SystemImagePart) Name() string {
> > - return systemImagePartName
> > + return fmt.Sprintf("ubuntu-%s", release.Flavor())
>
> Yay for this! Makes me wonder if we should move the "ubuntu-" string into release as well so that systemimage.go is a bit more agonstic about (but really minor minor).
>

Maybe so, but we also need to be backwards compatible here; if we can,
I'm all for it!

> > }
> >
> > // Origin returns the origin ("ubuntu")
>
>
> --
> https://code.launchpad.net/~sergiusens/snappy/rollYourOwnCorePersonal/+merge/264405
> You are the owner of lp:~sergiusens/snappy/rollYourOwnCorePersonal.

[Federico Gimenez (fgimenez)]

PASSED: Continuous integration, rev:570
http://10.55.60.183:8080/job/snappy-rolling-ci/25/
Executed test runs:

Click here to trigger a rebuild:
http://10.55.60.183:8080/job/snappy-rolling-ci/25/rebuild

review: Approve (continuous-integration)

Unmerged revisions

570. By Sergio Schvezov on 2015-07-10

Snappy to expose the core package as personal or core depending on the release

=== modified file 'cmd/snappy/cmd_info.go'
--- cmd/snappy/cmd_info.go	2015-07-01 14:56:49 +0000
+++ cmd/snappy/cmd_info.go	2015-07-10 11:47:46 +0000
@@ -26,6 +26,7 @@
 	"launchpad.net/snappy/i18n"
 	"launchpad.net/snappy/logger"
 	"launchpad.net/snappy/pkg"
+	"launchpad.net/snappy/release"
 	"launchpad.net/snappy/snappy"
 )
 
@@ -90,22 +91,12 @@
 	return nil
 }
 
-func ubuntuCoreChannel() string {
-	parts, err := snappy.ActiveSnapsByType(pkg.TypeCore)
-	if len(parts) == 1 && err == nil {
-		return parts[0].Channel()
-	}
-
-	return "unknown"
-}
-
 func info() error {
-	release := ubuntuCoreChannel()
 	frameworks, _ := snappy.ActiveSnapNamesByType(pkg.TypeFramework)
 	apps, _ := snappy.ActiveSnapNamesByType(pkg.TypeApp)
 
 	// TRANSLATORS: the %s release string
-	fmt.Printf(i18n.G("release: %s\n"), release)
+	fmt.Printf(i18n.G("release: %s\n"), release.String())
 	// TRANSLATORS: the %s an architecture string
 	fmt.Printf(i18n.G("architecture: %s\n"), snappy.Architecture())
 	// TRANSLATORS: the %s is a comma separated list of framework names

=== modified file 'release/release.go'
--- release/release.go	2015-05-15 13:33:27 +0000
+++ release/release.go	2015-07-10 11:47:46 +0000
@@ -30,9 +30,9 @@
 
 // Release contains a structure with the release information
 type Release struct {
-	Flavor  string
-	Series  string
-	Channel string
+	flavor  string
+	series  string
+	channel string
 }
 
 var rel Release
@@ -43,7 +43,7 @@
 
 // setLegacy is a helper to set the default initial release of 15.04-core
 func setLegacy() {
-	rel = Release{Flavor: "core", Series: "15.04"}
+	rel = Release{flavor: "core", series: "15.04"}
 }
 
 // String returns the release information in a string
@@ -51,9 +51,18 @@
 	return rel.String()
 }
 
+// Flavor returns the flavor for the release.
+func Flavor() string {
+	return rel.flavor
+}
+
 // Override sets up the release using a Release
-func Override(r Release) {
-	rel = r
+func Override(flavor, series, channel string) {
+	rel = Release{
+		flavor:  flavor,
+		series:  series,
+		channel: channel,
+	}
 }
 
 // Setup is used to initialiaze the release information for the system
@@ -84,9 +93,9 @@
 	}
 
 	rel = Release{
-		Flavor:  strings.Trim(channelParts[0], "ubuntu-"),
-		Series:  channelParts[1],
-		Channel: channelParts[2],
+		flavor:  strings.Trim(channelParts[0], "ubuntu-"),
+		series:  channelParts[1],
+		channel: channelParts[2],
 	}
 
 	return nil
@@ -95,5 +104,5 @@
 // String returns the release information in a string which is valid to
 // set for the store http headers.
 func (r Release) String() string {
-	return fmt.Sprintf("%s-%s", r.Series, r.Flavor)
+	return fmt.Sprintf("%s-%s", r.series, r.flavor)
 }

=== modified file 'release/release_test.go'
--- release/release_test.go	2015-06-02 20:53:10 +0000
+++ release/release_test.go	2015-07-10 11:47:46 +0000
@@ -59,17 +59,17 @@
 
 	c.Assert(Setup(s.root), IsNil)
 	c.Check(String(), Equals, "15.04-core")
-	c.Check(rel.Flavor, Equals, "core")
-	c.Check(rel.Series, Equals, "15.04")
-	c.Check(rel.Channel, Equals, "edge")
+	c.Check(rel.flavor, Equals, "core")
+	c.Check(rel.series, Equals, "15.04")
+	c.Check(rel.channel, Equals, "edge")
 }
 
 func (s *ReleaseTestSuite) TestOverride(c *C) {
-	Override(Release{Flavor: "personal", Series: "10.06", Channel: "beta"})
+	Override("personal", "10.06", "beta")
 	c.Check(String(), Equals, "10.06-personal")
-	c.Check(rel.Flavor, Equals, "personal")
-	c.Check(rel.Series, Equals, "10.06")
-	c.Check(rel.Channel, Equals, "beta")
+	c.Check(rel.flavor, Equals, "personal")
+	c.Check(rel.series, Equals, "10.06")
+	c.Check(rel.channel, Equals, "beta")
 }
 
 func (s *ReleaseTestSuite) TestSetupLegacyChannels(c *C) {
@@ -77,9 +77,9 @@
 
 	c.Assert(Setup(s.root), IsNil)
 	c.Check(String(), Equals, "15.04-core")
-	c.Check(rel.Flavor, Equals, "core")
-	c.Check(rel.Series, Equals, "15.04")
-	c.Check(rel.Channel, Equals, "")
+	c.Check(rel.flavor, Equals, "core")
+	c.Check(rel.series, Equals, "15.04")
+	c.Check(rel.channel, Equals, "")
 }
 
 func (s *ReleaseTestSuite) TestNoChannelErrors(c *C) {

=== modified file 'snappy/snapp_test.go'
--- snappy/snapp_test.go	2015-06-30 02:32:37 +0000
+++ snappy/snapp_test.go	2015-07-10 11:47:46 +0000
@@ -63,7 +63,7 @@
 	os.MkdirAll(snapServicesDir, 0755)
 	os.MkdirAll(snapSeccompDir, 0755)
 
-	release.Override(release.Release{Flavor: "core", Series: "15.04"})
+	release.Override("core", "15.04", "")
 
 	clickSystemHooksDir = filepath.Join(s.tempdir, "/usr/share/click/hooks")
 	os.MkdirAll(clickSystemHooksDir, 0755)

=== modified file 'snappy/systemimage.go'
--- snappy/systemimage.go	2015-07-01 14:48:33 +0000
+++ snappy/systemimage.go	2015-07-10 11:47:46 +0000
@@ -37,6 +37,7 @@
 	"launchpad.net/snappy/pkg"
 	"launchpad.net/snappy/progress"
 	"launchpad.net/snappy/provisioning"
+	"launchpad.net/snappy/release"
 )
 
 const (
@@ -97,7 +98,7 @@
 
 // Name returns the name
 func (s *SystemImagePart) Name() string {
-	return systemImagePartName
+	return fmt.Sprintf("ubuntu-%s", release.Flavor())
 }
 
 // Origin returns the origin ("ubuntu")
@@ -234,7 +235,7 @@
 	if err = s.partition.ToggleNextBoot(); err != nil {
 		return "", err
 	}
-	return systemImagePartName, nil
+	return s.Name(), nil
 }
 
 // Ensure the expected version update was applied to the expected partition.

=== modified file 'snappy/systemimage_test.go'
--- snappy/systemimage_test.go	2015-07-01 14:48:33 +0000
+++ snappy/systemimage_test.go	2015-07-10 11:47:46 +0000
@@ -30,6 +30,7 @@
 
 	"launchpad.net/snappy/partition"
 	"launchpad.net/snappy/provisioning"
+	"launchpad.net/snappy/release"
 
 	. "gopkg.in/check.v1"
 )
@@ -146,6 +147,20 @@
 	c.Assert(parts[0].DownloadSize(), Equals, int64(123166488))
 }
 
+func (s *SITestSuite) TestUpdateHasUpdateForPersonal(c *C) {
+	// change name
+	release.Override("personal", "rolling", "edge")
+
+	// add a update
+	mockSystemImageIndexJSON = fmt.Sprintf(mockSystemImageIndexJSONTemplate, "2")
+	parts, err := s.systemImage.Updates()
+	c.Assert(err, IsNil)
+	c.Assert(parts, HasLen, 1)
+	c.Assert(parts[0].Name(), Equals, "ubuntu-personal")
+	c.Assert(parts[0].Version(), Equals, "2")
+	c.Assert(parts[0].DownloadSize(), Equals, int64(123166488))
+}
+
 type MockPartition struct {
 	toggleNextBootCalled      bool
 	markBootSuccessfulCalled  bool