Improve newt upgrade

.github/newt_upgrade/allowed-ignored/expected.txt

@@ -1,7 +1,6 @@
 apache-mynewt-core
-apache-mynewt-mcumgr
 apache-mynewt-nimble
 arm-CMSIS_5
-mbedtls
 mcuboot
 nordic-nrfx
+stm-stm32g4xx_hal_driver

.github/newt_upgrade/allowed-ignored/project.yml

@@ -17,9 +17,6 @@
 # under the License.
 #
 
-project.repositories:
-   - apache-mynewt-core
-
 repository.apache-mynewt-core:
    type: github
    vers: 0-dev
@@ -33,16 +30,12 @@ repository.tinyusb:
    repo: tinyusb
 
 project.repositories.allowed:
-   - apache-mynewt-core
-   - apache-mynewt-nimble
-   - apache-mynewt-mcumgr
+   - apache-*
+   - ~stm-.*g4
    - mcuboot
    - arm-CMSIS_5
    - nordic-nrfx
-   - mbedtls
-   - stm-cmsis_device_f3
-   - stm-stm32f3xx_hal_driver
 
 project.repositories.ignored:
-   - stm-cmsis_device_f3
-   - stm-stm32f3xx_hal_driver
+   - apache-mynewt-mcumgr
+   - stm-cmsis_device_g4

.github/workflows/test_upgrade.yml

@@ -127,4 +127,4 @@ jobs:
         run: |
              echo "Test upgrade"
              newt upgrade
-             ls repos | diff -w expected.txt -
+             ls -1 repos | diff -w expected.txt -

newt/downloader/downloader.go

@@ -88,6 +88,12 @@ type Downloader interface {
 	// the repo is in a "detached head" state.
 	CurrentBranch(path string) (string, error)
 
+	// Retrieves commit SHA for current HEAD
+	CurrentHead() string
+
+	// Retrieves full SHA for given commit
+	CommitSha(path string, commit string) (string, error)
+
 	// LatestRc finds the commit of the latest release candidate.  It looks
 	// for commits with names matching the base commit string, but with with
 	// "_rc#" inserted.  This is useful when a release candidate is being
@@ -588,6 +594,13 @@ func (gd *GenericDownloader) HashFor(path string,
 		return c.hash, nil
 	}
 
+	if len(commit) < 40 {
+		sha, err := gd.CommitSha(path, commit)
+		if err == nil {
+			commit = sha
+		}
+	}
+
 	return commit, nil
 }
 
@@ -664,6 +677,20 @@ func (gd *GenericDownloader) CurrentBranch(path string) (string, error) {
 	return branch, nil
 }
 
+func (gd *GenericDownloader) CurrentHead() string {
+	return gd.head
+}
+
+func (gd *GenericDownloader) CommitSha(path string, commit string) (string, error) {
+	cmd := []string{"rev-parse", commit}
+	o, err := executeGitCommand(path, cmd, true)
+	if err != nil {
+		return "", err
+	}
+
+	return strings.TrimSpace(string(o)), nil
+}
+
 // Fetches the downloader's origin remote if it hasn't been fetched yet during
 // this run.
 func (gd *GenericDownloader) cachedFetch(fn func() error) error {

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 {
@@ -286,8 +289,7 @@ func (inst *Installer) filterUpgradeList(
 			}
 			curVer.Commit = ver.Commit
 			util.StatusMessage(util.VERBOSITY_DEFAULT,
-				"Skipping \"%s\": already upgraded (%s)\n",
-				name, curVer.String())
+				"Skipping %s, already upgraded to version %s\n", name, curVer.String())
 		}
 	}
 
@@ -345,6 +347,9 @@ func (inst *Installer) installPrompt(vm deprepo.VersionMap, op installOp,
 		return true, nil
 	}
 
+	if !ask {
+		return true, nil
+	}
 	util.StatusMessage(util.VERBOSITY_DEFAULT,
 		"Trying to make the following changes to the project:\n")
 
@@ -371,10 +376,6 @@ func (inst *Installer) installPrompt(vm deprepo.VersionMap, op installOp,
 		}
 	}
 
-	if !ask {
-		return true, nil
-	}
-
 	for {
 		fmt.Printf("Proceed? [Y/n] ")
 		line, more, err := bufio.NewReader(os.Stdin).ReadLine()
@@ -567,15 +568,12 @@ func (inst *Installer) Upgrade(candidates []*repo.Repo, force bool,
 			}
 		}
 
+		util.StatusMessage(util.VERBOSITY_DEFAULT,
+			"Upgrading %s to version %s\n", r.Name(), destVer.String())
+
 		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())
 	}
 
 	for _, r := range candidates {

newt/project/project.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -72,11 +73,11 @@ type Project struct {
 
 	// Contains names of repositories that will be upgraded.
 	// If it's empty all repos are allowed.
-	reposAllowed []string
+	reposAllowedRe []*regexp.Regexp
 
 	// Contains names of repositories that will be excluded from upgrade.
 	// Can override repositories from reposAllowed.
-	reposIgnored []string
+	reposIgnoredRe []*regexp.Regexp
 
 	// The local repository at the top-level of the project.  This repo is
 	// excluded from most repo operations.
@@ -181,10 +182,21 @@ func NewProject(dir string, download bool) (*Project, error) {
 	return proj, nil
 }
 
+func (proj *Project) patternsMatch(patterns *[]*regexp.Regexp, repoName string) bool {
+	for _, re := range *patterns {
+		if re.MatchString(repoName) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (proj *Project) isRepoAllowed(repoName string) bool {
-	if (len(proj.reposAllowed) == 0) || (util.SliceContains(proj.reposAllowed, repoName)) {
-		return !util.SliceContains(proj.reposIgnored, repoName)
+	if (len(proj.reposAllowedRe) == 0) || proj.patternsMatch(&proj.reposAllowedRe, repoName) {
+		return !proj.patternsMatch(&proj.reposIgnoredRe, repoName)
 	}
+
 	return false
 }
 
@@ -426,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,
@@ -443,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)
 	}
@@ -564,13 +575,33 @@ 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())
+			}
+
+			// No need to fetch if requested commit is already checked out
+			if r.IsHeadCommit(ver.Commit) {
+				continue
+			}
+		}
+
+		if _, err := r.UpdateDesc(); err != nil {
+			return err
 		}
 	}
 
@@ -641,6 +672,36 @@ func (proj *Project) addRepo(r *repo.Repo, download bool) error {
 	return nil
 }
 
+func (proj *Project) createRegexpPatterns(patterns []string) ([]*regexp.Regexp, error) {
+	var ret []*regexp.Regexp
+	var errLines []string
+
+	for _, pattern := range patterns {
+		var s string
+
+		if strings.HasPrefix(pattern, "~") {
+			s = "^" + pattern[1:]
+		} else if strings.HasSuffix(pattern, "*") {
+			s = "^" + pattern[:len(pattern)-1] + ".*$"
+		} else {
+			s = "^" + pattern + "$"
+		}
+
+		re, err := regexp.Compile(s)
+		if err != nil {
+			errLines = append(errLines, fmt.Sprintf("Invalid pattern: %s", pattern))
+		} else {
+			ret = append(ret, re)
+		}
+	}
+
+	if len(errLines) > 0 {
+		return ret, util.NewNewtError(strings.Join(errLines, "\n"))
+	} else {
+		return ret, nil
+	}
+}
+
 func (proj *Project) loadConfig(download bool) error {
 	yc, err := config.ReadFile(proj.BasePath + "/" + PROJECT_FILE_NAME)
 	if err != nil {
@@ -654,14 +715,19 @@ func (proj *Project) loadConfig(download bool) error {
 	proj.name, err = yc.GetValString("project.name", nil)
 	util.OneTimeWarningError(err)
 
-	proj.reposAllowed = make([]string, 0)
-	proj.reposAllowed, err = yc.GetValStringSlice("project.repositories.allowed", nil)
+	var reposAllowed []string
+	var reposIgnored []string
+
+	reposAllowed, err = yc.GetValStringSlice("project.repositories.allowed", nil)
+	util.OneTimeWarningError(err)
+	proj.reposAllowedRe, err = proj.createRegexpPatterns(reposAllowed)
 	util.OneTimeWarningError(err)
 
-	proj.reposIgnored = make([]string, 0)
-	proj.reposIgnored, err = yc.GetValStringSlice("project.repositories.ignored", nil)
+	reposIgnored, err = yc.GetValStringSlice("project.repositories.ignored", nil)
+	util.OneTimeWarningError(err)
+	reposIgnored = append(reposIgnored, newtutil.NewtIgnore...)
+	proj.reposIgnoredRe, err = proj.createRegexpPatterns(reposIgnored)
 	util.OneTimeWarningError(err)
-	proj.reposIgnored = append(proj.reposIgnored, newtutil.NewtIgnore...)
 
 	if !proj.isRepoAllowed("apache-mynewt-core") {
 		return util.NewNewtError("apache-mynewt-core repository must be allowed. " +
@@ -684,7 +750,7 @@ func (proj *Project) loadConfig(download bool) error {
 	// and try to load it.
 	for k, _ := range yc.AllSettings() {
 		repoName := strings.TrimPrefix(k, "repository.")
-		if repoName != k && !util.SliceContains(proj.reposIgnored, repoName) {
+		if repoName != k {
 			fields, err := yc.GetValStringMapString(k, nil)
 			util.OneTimeWarningError(err)
 
@@ -707,7 +773,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
 			}