Merge lp:~chipaca/snappy/no-empty-channel-in-update into lp:~snappy-dev/snappy/snappy-moved-to-github

[John Lenton]

Don't include empty channels in the bulk POST.

[Sergio Schvezov (sergiusens)]

The code looks good, I just don't understand one thing

review: Needs Information

[Sergio Schvezov (sergiusens)]

review: Approve

[Michael Vogt (mvo)]

Looks good, thanks!

review: Approve

=== modified file 'snappy/click_test.go'
--- snappy/click_test.go	2015-10-16 16:35:37 +0000
+++ snappy/click_test.go	2015-10-26 16:00:20 +0000
@@ -523,7 +523,7 @@
 vendor: Foo Bar <[e-mail]>`)
 	_, err := installClick(snapFile, AllowOEM, nil, testOrigin)
 	c.Assert(err, IsNil)
-	c.Assert(storeMinimalRemoteManifest("foo", "foo", testOrigin, "1.0", ""), IsNil)
+	c.Assert(storeMinimalRemoteManifest("foo", "foo", testOrigin, "1.0", "", "remote-channel"), IsNil)
 
 	contentFile := filepath.Join(s.tempdir, "oem", "foo", "1.0", "bin", "foo")
 	_, err = os.Stat(contentFile)
@@ -539,7 +539,7 @@
 vendor: Foo Bar <[e-mail]>`)
 	_, err = installClick(snapFile, 0, nil, testOrigin)
 	c.Check(err, IsNil)
-	c.Assert(storeMinimalRemoteManifest("foo", "foo", testOrigin, "2.0", ""), IsNil)
+	c.Assert(storeMinimalRemoteManifest("foo", "foo", testOrigin, "2.0", "", "remote-channel"), IsNil)
 
 	// XXX: I think this next test now tests something we actually don't
 	// want. At least for fwks and apps, sideloading something installed

=== modified file 'snappy/common_test.go'
--- snappy/common_test.go	2015-10-15 08:26:11 +0000
+++ snappy/common_test.go	2015-10-26 16:00:20 +0000
@@ -92,14 +92,14 @@
 		return "", err
 	}
 
-	if err := storeMinimalRemoteManifest(dirName, m.Name, testOrigin, m.Version, "Hello"); err != nil {
+	if err := storeMinimalRemoteManifest(dirName, m.Name, testOrigin, m.Version, "Hello", "remote-channel"); err != nil {
 		return "", err
 	}
 
 	return yamlFile, nil
 }
 
-func storeMinimalRemoteManifest(qn, name, origin, version, desc string) error {
+func storeMinimalRemoteManifest(qn, name, origin, version, desc, channel string) error {
 	if origin == SideloadedOrigin {
 		panic("store remote manifest for sideloaded package")
 	}
@@ -108,7 +108,7 @@
 		Origin:      origin,
 		Version:     version,
 		Description: desc,
-		Channel:     "remote-channel",
+		Channel:     channel,
 	})
 	if err != nil {
 		return err
@@ -205,13 +205,13 @@
 	n, err := installClick(snapFile, AllowUnauthenticated|AllowOEM, inter, testOrigin)
 	c.Assert(err, IsNil)
 	c.Assert(n, Equals, "foo")
-	c.Assert(storeMinimalRemoteManifest(qn, "foo", testOrigin, "1.0", ""), IsNil)
+	c.Assert(storeMinimalRemoteManifest(qn, "foo", testOrigin, "1.0", "", "remote-channel"), IsNil)
 
 	snapFile = makeTestSnapPackage(c, packageYaml+"version: 2.0")
 	n, err = installClick(snapFile, AllowUnauthenticated|AllowOEM, inter, testOrigin)
 	c.Assert(err, IsNil)
 	c.Assert(n, Equals, "foo")
-	c.Assert(storeMinimalRemoteManifest(qn, "foo", testOrigin, "2.0", ""), IsNil)
+	c.Assert(storeMinimalRemoteManifest(qn, "foo", testOrigin, "2.0", "", "remote-channel"), IsNil)
 
 	m := NewMetaRepository()
 	installed, err := m.Installed()

=== modified file 'snappy/parts.go'
--- snappy/parts.go	2015-10-20 06:43:30 +0000
+++ snappy/parts.go	2015-10-26 16:00:20 +0000
@@ -64,6 +64,18 @@
 	return p.Name() + "." + p.Origin()
 }
 
+// FullNameWithChannel returns the FullName, with the channel appended
+// if it has one.
+func fullNameWithChannel(p Part) string {
+	name := FullName(p)
+	ch := p.Channel()
+	if ch == "" {
+		return name
+	}
+
+	return fmt.Sprintf("%s/%s", name, ch)
+}
+
 // Part representation of a snappy part
 type Part interface {
 

=== modified file 'snappy/parts_test.go'
--- snappy/parts_test.go	2015-10-10 13:27:50 +0000
+++ snappy/parts_test.go	2015-10-26 16:00:20 +0000
@@ -85,6 +85,20 @@
 		{QualifiedName, pkg.TypeFramework, "fwk"},
 		{FullName, pkg.TypeApp, "app." + testOrigin},
 		{FullName, pkg.TypeFramework, "fwk." + testOrigin},
+		{fullNameWithChannel, pkg.TypeApp, "app." + testOrigin + "/remote-channel"},
+		{fullNameWithChannel, pkg.TypeFramework, "fwk." + testOrigin + "/remote-channel"},
+	} {
+		names, err := ActiveSnapIterByType(t.f, t.t)
+		c.Check(err, IsNil)
+		c.Check(names, DeepEquals, []string{t.n})
+	}
+
+	// now remove the channel
+	storeMinimalRemoteManifest("app."+testOrigin, "app", testOrigin, "1.10", "Hello.", "")
+	storeMinimalRemoteManifest("fwk", "fwk", testOrigin, "1.0", "Hello.", "")
+	for _, t := range []T{
+		{fullNameWithChannel, pkg.TypeApp, "app." + testOrigin},
+		{fullNameWithChannel, pkg.TypeFramework, "fwk." + testOrigin},
 	} {
 		names, err := ActiveSnapIterByType(t.f, t.t)
 		c.Check(err, IsNil)

=== modified file 'snappy/snapp.go'
--- snappy/snapp.go	2015-10-15 08:26:11 +0000
+++ snappy/snapp.go	2015-10-26 16:00:20 +0000
@@ -1908,9 +1908,6 @@
 	// sense in sending it our ubuntu-core snap
 	//
 	// NOTE this *will* send .sideload apps to the store.
-	fullNameWithChannel := func(p Part) string {
-		return fmt.Sprintf("%s/%s", FullName(p), p.Channel())
-	}
 	installed, err := ActiveSnapIterByType(fullNameWithChannel, pkg.TypeApp, pkg.TypeFramework, pkg.TypeOem)
 	if err != nil || len(installed) == 0 {
 		return nil, err