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

Proposed by Sergio Schvezov on 2015-07-10
Status: Needs review
Proposed branch: lp:~sergiusens/snappy/rollYourOwnCorePersonal
Merge into: lp:~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 220 lines (+50/-34)
6 files modified
cmd/snappy/cmd_info.go (+2/-11)
release/release.go (+19/-10)
release/release_test.go (+10/-10)
snappy/snapp_test.go (+1/-1)
snappy/systemimage.go (+3/-2)
snappy/systemimage_test.go (+15/-0)
To merge this branch: bzr merge lp:~sergiusens/snappy/rollYourOwnCorePersonal
Reviewer Review Type Date Requested Status
Federico Gimenez (community) continuous-integration Approve on 2015-08-19
Michael Vogt 2015-07-10 Needs Information on 2015-07-13
Snappy Tarmac continuous-integration Pending
Review via email: mp+264405@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Michael Vogt (mvo) wrote :

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

review: Needs Information
Sergio Schvezov (sergiusens) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/snappy/cmd_info.go'
2--- cmd/snappy/cmd_info.go 2015-07-01 14:56:49 +0000
3+++ cmd/snappy/cmd_info.go 2015-07-10 11:47:46 +0000
4@@ -26,6 +26,7 @@
5 "launchpad.net/snappy/i18n"
6 "launchpad.net/snappy/logger"
7 "launchpad.net/snappy/pkg"
8+ "launchpad.net/snappy/release"
9 "launchpad.net/snappy/snappy"
10 )
11
12@@ -90,22 +91,12 @@
13 return nil
14 }
15
16-func ubuntuCoreChannel() string {
17- parts, err := snappy.ActiveSnapsByType(pkg.TypeCore)
18- if len(parts) == 1 && err == nil {
19- return parts[0].Channel()
20- }
21-
22- return "unknown"
23-}
24-
25 func info() error {
26- release := ubuntuCoreChannel()
27 frameworks, _ := snappy.ActiveSnapNamesByType(pkg.TypeFramework)
28 apps, _ := snappy.ActiveSnapNamesByType(pkg.TypeApp)
29
30 // TRANSLATORS: the %s release string
31- fmt.Printf(i18n.G("release: %s\n"), release)
32+ fmt.Printf(i18n.G("release: %s\n"), release.String())
33 // TRANSLATORS: the %s an architecture string
34 fmt.Printf(i18n.G("architecture: %s\n"), snappy.Architecture())
35 // TRANSLATORS: the %s is a comma separated list of framework names
36
37=== modified file 'release/release.go'
38--- release/release.go 2015-05-15 13:33:27 +0000
39+++ release/release.go 2015-07-10 11:47:46 +0000
40@@ -30,9 +30,9 @@
41
42 // Release contains a structure with the release information
43 type Release struct {
44- Flavor string
45- Series string
46- Channel string
47+ flavor string
48+ series string
49+ channel string
50 }
51
52 var rel Release
53@@ -43,7 +43,7 @@
54
55 // setLegacy is a helper to set the default initial release of 15.04-core
56 func setLegacy() {
57- rel = Release{Flavor: "core", Series: "15.04"}
58+ rel = Release{flavor: "core", series: "15.04"}
59 }
60
61 // String returns the release information in a string
62@@ -51,9 +51,18 @@
63 return rel.String()
64 }
65
66+// Flavor returns the flavor for the release.
67+func Flavor() string {
68+ return rel.flavor
69+}
70+
71 // Override sets up the release using a Release
72-func Override(r Release) {
73- rel = r
74+func Override(flavor, series, channel string) {
75+ rel = Release{
76+ flavor: flavor,
77+ series: series,
78+ channel: channel,
79+ }
80 }
81
82 // Setup is used to initialiaze the release information for the system
83@@ -84,9 +93,9 @@
84 }
85
86 rel = Release{
87- Flavor: strings.Trim(channelParts[0], "ubuntu-"),
88- Series: channelParts[1],
89- Channel: channelParts[2],
90+ flavor: strings.Trim(channelParts[0], "ubuntu-"),
91+ series: channelParts[1],
92+ channel: channelParts[2],
93 }
94
95 return nil
96@@ -95,5 +104,5 @@
97 // String returns the release information in a string which is valid to
98 // set for the store http headers.
99 func (r Release) String() string {
100- return fmt.Sprintf("%s-%s", r.Series, r.Flavor)
101+ return fmt.Sprintf("%s-%s", r.series, r.flavor)
102 }
103
104=== modified file 'release/release_test.go'
105--- release/release_test.go 2015-06-02 20:53:10 +0000
106+++ release/release_test.go 2015-07-10 11:47:46 +0000
107@@ -59,17 +59,17 @@
108
109 c.Assert(Setup(s.root), IsNil)
110 c.Check(String(), Equals, "15.04-core")
111- c.Check(rel.Flavor, Equals, "core")
112- c.Check(rel.Series, Equals, "15.04")
113- c.Check(rel.Channel, Equals, "edge")
114+ c.Check(rel.flavor, Equals, "core")
115+ c.Check(rel.series, Equals, "15.04")
116+ c.Check(rel.channel, Equals, "edge")
117 }
118
119 func (s *ReleaseTestSuite) TestOverride(c *C) {
120- Override(Release{Flavor: "personal", Series: "10.06", Channel: "beta"})
121+ Override("personal", "10.06", "beta")
122 c.Check(String(), Equals, "10.06-personal")
123- c.Check(rel.Flavor, Equals, "personal")
124- c.Check(rel.Series, Equals, "10.06")
125- c.Check(rel.Channel, Equals, "beta")
126+ c.Check(rel.flavor, Equals, "personal")
127+ c.Check(rel.series, Equals, "10.06")
128+ c.Check(rel.channel, Equals, "beta")
129 }
130
131 func (s *ReleaseTestSuite) TestSetupLegacyChannels(c *C) {
132@@ -77,9 +77,9 @@
133
134 c.Assert(Setup(s.root), IsNil)
135 c.Check(String(), Equals, "15.04-core")
136- c.Check(rel.Flavor, Equals, "core")
137- c.Check(rel.Series, Equals, "15.04")
138- c.Check(rel.Channel, Equals, "")
139+ c.Check(rel.flavor, Equals, "core")
140+ c.Check(rel.series, Equals, "15.04")
141+ c.Check(rel.channel, Equals, "")
142 }
143
144 func (s *ReleaseTestSuite) TestNoChannelErrors(c *C) {
145
146=== modified file 'snappy/snapp_test.go'
147--- snappy/snapp_test.go 2015-06-30 02:32:37 +0000
148+++ snappy/snapp_test.go 2015-07-10 11:47:46 +0000
149@@ -63,7 +63,7 @@
150 os.MkdirAll(snapServicesDir, 0755)
151 os.MkdirAll(snapSeccompDir, 0755)
152
153- release.Override(release.Release{Flavor: "core", Series: "15.04"})
154+ release.Override("core", "15.04", "")
155
156 clickSystemHooksDir = filepath.Join(s.tempdir, "/usr/share/click/hooks")
157 os.MkdirAll(clickSystemHooksDir, 0755)
158
159=== modified file 'snappy/systemimage.go'
160--- snappy/systemimage.go 2015-07-01 14:48:33 +0000
161+++ snappy/systemimage.go 2015-07-10 11:47:46 +0000
162@@ -37,6 +37,7 @@
163 "launchpad.net/snappy/pkg"
164 "launchpad.net/snappy/progress"
165 "launchpad.net/snappy/provisioning"
166+ "launchpad.net/snappy/release"
167 )
168
169 const (
170@@ -97,7 +98,7 @@
171
172 // Name returns the name
173 func (s *SystemImagePart) Name() string {
174- return systemImagePartName
175+ return fmt.Sprintf("ubuntu-%s", release.Flavor())
176 }
177
178 // Origin returns the origin ("ubuntu")
179@@ -234,7 +235,7 @@
180 if err = s.partition.ToggleNextBoot(); err != nil {
181 return "", err
182 }
183- return systemImagePartName, nil
184+ return s.Name(), nil
185 }
186
187 // Ensure the expected version update was applied to the expected partition.
188
189=== modified file 'snappy/systemimage_test.go'
190--- snappy/systemimage_test.go 2015-07-01 14:48:33 +0000
191+++ snappy/systemimage_test.go 2015-07-10 11:47:46 +0000
192@@ -30,6 +30,7 @@
193
194 "launchpad.net/snappy/partition"
195 "launchpad.net/snappy/provisioning"
196+ "launchpad.net/snappy/release"
197
198 . "gopkg.in/check.v1"
199 )
200@@ -146,6 +147,20 @@
201 c.Assert(parts[0].DownloadSize(), Equals, int64(123166488))
202 }
203
204+func (s *SITestSuite) TestUpdateHasUpdateForPersonal(c *C) {
205+ // change name
206+ release.Override("personal", "rolling", "edge")
207+
208+ // add a update
209+ mockSystemImageIndexJSON = fmt.Sprintf(mockSystemImageIndexJSONTemplate, "2")
210+ parts, err := s.systemImage.Updates()
211+ c.Assert(err, IsNil)
212+ c.Assert(parts, HasLen, 1)
213+ c.Assert(parts[0].Name(), Equals, "ubuntu-personal")
214+ c.Assert(parts[0].Version(), Equals, "2")
215+ c.Assert(parts[0].DownloadSize(), Equals, int64(123166488))
216+}
217+
218 type MockPartition struct {
219 toggleNextBootCalled bool
220 markBootSuccessfulCalled bool

Subscribers

People subscribed via source and target branches