Improve newt upgrade

This improves newt upgrade by optimizing the way repos are checked for
potential upgrade. Each repo is now checked only once i.e. if it was
loaded from project.yml and updated it's not considered for update
anymore. This also means no duplicate messages are printed.

Author: andrzej-kaczmarek

newt/install/install.go

@@ -245,10 +245,6 @@ func (inst *Installer) shouldUpgradeRepo(
 		return true, nil
 	}
 
-	if r.IsUpgradedFromProjectYml() {
-		return false, nil
-	}
-
 	if !r.VersionsEqual(*curVer, destVer) {
 		return true, nil
 	}
@@ -269,6 +265,13 @@ func (inst *Installer) filterUpgradeList(
 	filtered := deprepo.VersionMap{}
 
 	for _, name := range vm.SortedNames() {
+		r := inst.repos[name]
+		if r.IsCheckedForUpgrade() {
+			continue
+		}
+
+		inst.repos[name].SetCheckedForUpgrade()
+
 		ver := vm[name]
 		doUpgrade, err := inst.shouldUpgradeRepo(name, ver)
 		if err != nil {
@@ -570,9 +573,7 @@ func (inst *Installer) Upgrade(candidates []*repo.Repo, force bool,
 		if err := r.Upgrade(destVer); err != nil {
 			return err
 		}
-		if r.IsFromProjectYml() {
-			r.SetIsUpgradedFromProjectYml()
-		}
+
 		util.StatusMessage(util.VERBOSITY_DEFAULT,
 			"%s successfully upgraded to version %s\n",
 			r.Name(), destVer.String())

newt/project/project.go

@@ -438,6 +438,9 @@ func (proj *Project) InfoIf(predicate func(r *repo.Repo) bool,
 func (proj *Project) loadRepo(name string, fields map[string]string) (
 	*repo.Repo, error) {
 
+	if !proj.isRepoAllowed(name) {
+		return nil, nil
+	}
 	// First, read the repo description from the supplied fields.
 	if fields["type"] == "" {
 		return nil,
@@ -455,10 +458,6 @@ func (proj *Project) loadRepo(name string, fields map[string]string) (
 		return nil, err
 	}
 
-	if !proj.isRepoAllowed(r.Name()) {
-		return nil, nil
-	}
-
 	for _, ignDir := range ignoreSearchDirs {
 		r.AddIgnoreDir(ignDir)
 	}
@@ -576,13 +575,28 @@ func (proj *Project) downloadRepositoryYmlFiles() error {
 	// Download the `repository.yml` file for each root-level repo (those
 	// specified in the `project.yml` file).
 	for _, r := range proj.repos.Sorted() {
-		if !r.IsLocal() && !r.IsExternal(r.Path()) {
-			if _, err := r.UpdateDesc(); err != nil {
-				return err
-			}
+		if r.IsUpdated() {
+			continue
 		}
+
+		if r.IsLocal() {
+			continue
+		}
+
 		if r.IsExternal(r.Path()) {
-			r.Downloader().Fetch(r.Path())
+			ver := proj.rootRepoReqs[r.Name()]
+
+			// External repositories can only use commit stability since they do
+			// not have repository.yml
+			if len(ver.Commit) == 0 {
+				return util.FmtNewtError(
+					"External repository \"%s\" does not specify valid commit version (%s)",
+					r.Name(), ver.String())
+			}
+		}
+
+		if _, err := r.UpdateDesc(); err != nil {
+			return err
 		}
 	}
 
@@ -754,7 +768,7 @@ func (proj *Project) loadConfig(download bool) error {
 						"%s (%s)",
 					repoName, fields["vers"], err.Error())
 			}
-			r.SetIsFromProjectYml()
+
 			if err := proj.addRepo(r, download); err != nil {
 				return err
 			}

newt/repo/repo.go

@@ -55,6 +55,10 @@ type Repo struct {
 	local      bool
 	ncMap      compat.NewtCompatMap
 
+	// Indicates if repo was checked for potential upgrade. This avoids checking
+	// the same repo multiple times in different phases
+	upgradeChecked bool
+
 	// If repo was added from package (not from project.yml), this variable stores name of this package
 	pkgName string
 
@@ -67,14 +71,6 @@ type Repo struct {
 	// version => commit
 	vers map[newtutil.RepoVersion]string
 
-	// Since we are calling upgrade twice - first time for repos from project.yml
-	// and second time for repos from packages, we need to keep track on status
-	// of each repo. If repo is defined in project.yml we want to upgrade it only
-	// once so the version from project.yml stays valid. These flags are used for
-	// this purpose.
-	isFromProjectYml         bool
-	isUpgradedFromProjectYml bool
-
 	hasSubmodules bool
 	submodules    []string
 
@@ -227,22 +223,6 @@ func (r *Repo) patchesFilePath() string {
 		"/.patches/"
 }
 
-func (r *Repo) IsUpgradedFromProjectYml() bool {
-	return r.isUpgradedFromProjectYml
-}
-
-func (r *Repo) SetIsUpgradedFromProjectYml() {
-	r.isUpgradedFromProjectYml = true
-}
-
-func (r *Repo) IsFromProjectYml() bool {
-	return r.isFromProjectYml
-}
-
-func (r *Repo) SetIsFromProjectYml() {
-	r.isFromProjectYml = true
-}
-
 // Checks for repository.yml file presence in specified repo folder.
 // If there is no such file, the repo in specified folder is external.
 func (r *Repo) IsExternal(dir string) bool {
@@ -411,21 +391,39 @@ func (r *Repo) UpdateDesc() (bool, error) {
 		return false, err
 	}
 
-	// Download `repository.yml`.
-	if err := r.DownloadDesc(); err != nil {
-		return false, err
-	}
+	if r.IsExternal(r.Path()) {
+		if err := r.downloader.Fetch(r.Path()); err != nil {
+			return false, err
+		}
+	} else {
+		// Download `repository.yml`.
+		if err := r.DownloadDesc(); err != nil {
+			return false, err
+		}
 
-	// Read `repository.yml` and populate this repo object.
-	if err := r.Read(); err != nil {
-		return false, err
+		// Read `repository.yml` and populate this repo object.
+		if err := r.Read(); err != nil {
+			return false, err
+		}
 	}
 
 	r.updated = true
 
 	return true, nil
 }
 
+func (r *Repo) IsUpdated() bool {
+	return r.updated
+}
+
+func (r *Repo) SetCheckedForUpgrade() {
+	r.upgradeChecked = true
+}
+
+func (r *Repo) IsCheckedForUpgrade() bool {
+	return r.upgradeChecked
+}
+
 func (r *Repo) EnsureExists() error {
 	// Clone the repo if it doesn't exist.
 	if !r.CheckExists() {