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

[Sergio Schvezov]

Refactor Install to take a progress.Meter

[lp:~sergiusens/snappy/installFactorX]

340. By Sergio Schvezov on 2015-04-13

Renaming variables

341. By Sergio Schvezov on 2015-04-13

Removing panic

[John Lenton (chipaca)]

review: Approve

=== modified file 'cmd/snappy/cmd_install.go'
--- cmd/snappy/cmd_install.go	2015-04-13 02:35:57 +0000
+++ cmd/snappy/cmd_install.go	2015-04-13 22:40:19 +0000
@@ -23,6 +23,7 @@
 	"os"
 
 	"launchpad.net/snappy/priv"
+	"launchpad.net/snappy/progress"
 	"launchpad.net/snappy/snappy"
 )
 
@@ -44,11 +45,11 @@
 }
 
 func (x *cmdInstall) Execute(args []string) (err error) {
-	possiblepkgName := x.Positional.PackageName
+	pkgName := x.Positional.PackageName
 	configFile := x.Positional.ConfigFile
 
 	// FIXME patch goflags to allow for specific n required positional arguments
-	if possiblepkgName == "" {
+	if pkgName == "" {
 		return errors.New("package name is required")
 	}
 
@@ -66,18 +67,18 @@
 		flags |= snappy.AllowUnauthenticated
 	}
 
-	fmt.Printf("Installing %s\n", possiblepkgName)
-	var pkgName string
+	fmt.Printf("Installing %s\n", pkgName)
 
-	pkgName, err = snappy.Install(possiblepkgName, flags)
+	pbar := progress.NewTextProgress(pkgName)
+	realPkgName, err := snappy.Install(pkgName, flags, pbar)
 	if err == snappy.ErrPackageNotFound {
-		return fmt.Errorf("No package '%s' for %s", possiblepkgName, ubuntuCoreChannel())
+		return fmt.Errorf("No package '%s' for %s", pkgName, ubuntuCoreChannel())
 	} else if err != nil {
 		return err
 	}
 
 	if configFile != "" {
-		if _, err := configurePackage(pkgName, configFile); err != nil {
+		if _, err := configurePackage(realPkgName, configFile); err != nil {
 			return err
 		}
 	}

=== modified file 'progress/progress.go'
--- progress/progress.go	2015-04-07 11:43:27 +0000
+++ progress/progress.go	2015-04-13 22:40:19 +0000
@@ -75,7 +75,7 @@
 
 // Write does nothing
 func (t *NullProgress) Write(p []byte) (n int, err error) {
-	return n, nil
+	return len(p), nil
 }
 
 // Notify does nothing

=== modified file 'snappy/install.go'
--- snappy/install.go	2015-04-13 20:52:12 +0000
+++ snappy/install.go	2015-04-13 22:40:19 +0000
@@ -64,8 +64,8 @@
 
 // Install the givens snap names provided via args. This can be local
 // files or snaps that are queried from the store
-func Install(name string, flags InstallFlags) (string, error) {
-	name, err := doInstall(name, flags)
+func Install(name string, flags InstallFlags, meter progress.Meter) (string, error) {
+	name, err := doInstall(name, flags, meter)
 	if err != nil {
 		return "", logger.LogError(err)
 	}
@@ -73,7 +73,7 @@
 	return name, logger.LogError(GarbageCollect(name, flags))
 }
 
-func doInstall(name string, flags InstallFlags) (string, error) {
+func doInstall(name string, flags InstallFlags, meter progress.Meter) (string, error) {
 	// consume local parts
 	if fi, err := os.Stat(name); err == nil && fi.Mode().IsRegular() {
 		// we allow unauthenticated package when in developer
@@ -82,27 +82,23 @@
 			flags |= AllowUnauthenticated
 		}
 
-		pbar := progress.NewTextProgress(name)
-		return installClick(name, flags, pbar)
+		return installClick(name, flags, meter)
 	}
 
 	// check repos next
-	m := NewMetaRepository()
-	found, _ := m.Details(name)
+	mStore := NewMetaStoreRepository()
+	installed, err := NewMetaLocalRepository().Installed()
+	if err != nil {
+		return "", err
+	}
+
+	found, _ := mStore.Details(name)
 	for _, part := range found {
-		// act only on parts that are downloadable
-		if !part.IsInstalled() {
-			installed, err := m.Installed()
-			if err != nil {
-				return "", err
-			}
-			cur := FindSnapByNameAndVersion(part.Name(), part.Version(), installed)
-			if cur != nil {
-				return "", ErrAlreadyInstalled
-			}
-			pbar := progress.NewTextProgress(part.Name())
-			return part.Install(pbar, flags)
+		cur := FindSnapByNameAndVersion(part.Name(), part.Version(), installed)
+		if cur != nil {
+			return "", ErrAlreadyInstalled
 		}
+		return part.Install(meter, flags)
 	}
 
 	return "", ErrPackageNotFound

=== modified file 'snappy/install_test.go'
--- snappy/install_test.go	2015-04-13 02:35:57 +0000
+++ snappy/install_test.go	2015-04-13 22:40:19 +0000
@@ -27,6 +27,7 @@
 
 	. "launchpad.net/gocheck"
 	"launchpad.net/snappy/helpers"
+	"launchpad.net/snappy/progress"
 )
 
 func makeCloudInitMetaData(c *C, content string) string {
@@ -54,7 +55,7 @@
 
 func (s *SnapTestSuite) TestInstallInstall(c *C) {
 	snapFile := makeTestSnapPackage(c, "")
-	name, err := Install(snapFile, AllowUnauthenticated|DoInstallGC)
+	name, err := Install(snapFile, AllowUnauthenticated|DoInstallGC, &progress.NullProgress{})
 	c.Assert(err, IsNil)
 	c.Check(name, Equals, "foo")
 }
@@ -71,15 +72,15 @@
 vendor: Foo Bar <[e-mail]>
 `
 	snapFile := makeTestSnapPackage(c, packageYaml+"version: 1.0")
-	_, err = Install(snapFile, flags)
+	_, err = Install(snapFile, flags, &progress.NullProgress{})
 	c.Assert(err, IsNil)
 
 	snapFile = makeTestSnapPackage(c, packageYaml+"version: 2.0")
-	_, err = Install(snapFile, flags)
+	_, err = Install(snapFile, flags, &progress.NullProgress{})
 	c.Assert(err, IsNil)
 
 	snapFile = makeTestSnapPackage(c, packageYaml+"version: 3.0")
-	_, err = Install(snapFile, flags)
+	_, err = Install(snapFile, flags, &progress.NullProgress{})
 	c.Assert(err, IsNil)
 }
 
@@ -129,15 +130,10 @@
 	c.Assert(mockServer, NotNil)
 	defer mockServer.Close()
 
-	// ugh, progress bars from tests
-	stdout := os.Stdout
-	defer func() { os.Stdout = stdout }()
-	os.Stdout = nil
-
-	name, err := Install("foo", 0)
+	name, err := Install("foo", 0, &progress.NullProgress{})
 	c.Assert(err, IsNil)
 	c.Check(name, Equals, "foo")
 
-	_, err = Install("foo", 0)
+	_, err = Install("foo", 0, &progress.NullProgress{})
 	c.Assert(err, Equals, ErrAlreadyInstalled)
 }

=== modified file 'snappy/parts.go'
--- snappy/parts.go	2015-04-13 03:48:11 +0000
+++ snappy/parts.go	2015-04-13 22:40:19 +0000
@@ -112,6 +112,36 @@
 	all []Repository
 }
 
+// NewMetaStoreRepository returns a MetaRepository of stores
+func NewMetaStoreRepository() *MetaRepository {
+	m := new(MetaRepository)
+	m.all = []Repository{}
+
+	if repo := NewUbuntuStoreSnapRepository(); repo != nil {
+		m.all = append(m.all, repo)
+	}
+
+	return m
+}
+
+// NewMetaLocalRepository returns a MetaRepository of stores
+func NewMetaLocalRepository() *MetaRepository {
+	m := new(MetaRepository)
+	m.all = []Repository{}
+
+	if repo := NewSystemImageRepository(); repo != nil {
+		m.all = append(m.all, repo)
+	}
+	if repo := NewLocalSnapRepository(snapAppsDir); repo != nil {
+		m.all = append(m.all, repo)
+	}
+	if repo := NewLocalSnapRepository(snapOemDir); repo != nil {
+		m.all = append(m.all, repo)
+	}
+
+	return m
+}
+
 // NewMetaRepository returns a new MetaRepository
 func NewMetaRepository() *MetaRepository {
 	// FIXME: make this a configuration file