You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2018/09/19 22:23:17 UTC

[GitHub] ccollins476ad closed pull request #206: Improve `newt info` command; show repo ver

ccollins476ad closed pull request #206: Improve `newt info` command; show repo ver
URL: https://github.com/apache/mynewt-newt/pull/206
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/newt/cli/project_cmds.go b/newt/cli/project_cmds.go
index d54fdeae..42045249 100644
--- a/newt/cli/project_cmds.go
+++ b/newt/cli/project_cmds.go
@@ -33,6 +33,8 @@ import (
 	"mynewt.apache.org/newt/util"
 )
 
+var infoRemote bool
+
 func newRunCmd(cmd *cobra.Command, args []string) {
 	if len(args) < 1 {
 		NewtUsage(cmd, util.NewNewtError("Must specify "+
@@ -91,7 +93,7 @@ func makeRepoPredicate(repoNames []string) func(r *repo.Repo) bool {
 	return func(r *repo.Repo) bool {
 		if !r.IsLocal() {
 			for _, arg := range repoNames {
-				if r.Name() == arg {
+				if strings.TrimPrefix(r.Name(), "@") == arg {
 					return true
 				}
 			}
@@ -125,12 +127,20 @@ func upgradeRunCmd(cmd *cobra.Command, args []string) {
 }
 
 func infoRunCmd(cmd *cobra.Command, args []string) {
-	reqRepoName := ""
-	if len(args) >= 1 {
-		reqRepoName = strings.TrimPrefix(args[0], "@")
+	proj := TryGetProject()
+
+	// If no arguments specified, print status of all installed repos.
+	if len(args) == 0 {
+		pred := func(r *repo.Repo) bool { return !r.IsLocal() }
+
+		if err := proj.InfoIf(pred, infoRemote); err != nil {
+			NewtUsage(nil, err)
+		}
+		return
 	}
 
-	proj := TryGetProject()
+	// Otherwise, list packages specified repo contains.
+	reqRepoName := strings.TrimPrefix(args[0], "@")
 
 	repoNames := []string{}
 	for repoName, _ := range proj.PackageList() {
@@ -138,19 +148,6 @@ func infoRunCmd(cmd *cobra.Command, args []string) {
 	}
 	sort.Strings(repoNames)
 
-	if reqRepoName == "" {
-		util.StatusMessage(util.VERBOSITY_DEFAULT, "Repositories in %s:\n",
-			proj.Name())
-
-		for _, repoName := range repoNames {
-			util.StatusMessage(util.VERBOSITY_DEFAULT, "    * @%s\n", repoName)
-		}
-
-		// Now display the packages in the local repository.
-		util.StatusMessage(util.VERBOSITY_DEFAULT, "\n")
-		reqRepoName = "local"
-	}
-
 	firstRepo := true
 	for _, repoName := range repoNames {
 		if reqRepoName == "all" || reqRepoName == repoName {
@@ -276,6 +273,9 @@ func AddProjectCommands(cmd *cobra.Command) {
 		Example: infoHelpEx,
 		Run:     infoRunCmd,
 	}
+	infoCmd.PersistentFlags().BoolVarP(&infoRemote,
+		"remote", "r", false,
+		"Fetch latest repos to determine if upgrades are required")
 
 	cmd.AddCommand(infoCmd)
 }
diff --git a/newt/downloader/downloader.go b/newt/downloader/downloader.go
index c0e9afe2..58d8cda9 100644
--- a/newt/downloader/downloader.go
+++ b/newt/downloader/downloader.go
@@ -59,8 +59,8 @@ type Downloader interface {
 	// Fetches all remotes and merges the specified branch into the local repo.
 	Pull(path string, branchName string) error
 
-	// Indicates whether there are any uncommitted changes to the repo.
-	AreChanges(path string) (bool, error)
+	// Indicates whether the repo is in a clean or dirty state.
+	DirtyState(path string) (string, error)
 
 	// Determines the type of the specified commit.
 	CommitType(path string, commit string) (DownloaderCommitType, error)
@@ -311,20 +311,6 @@ func commitType(repoDir string, commit string) (DownloaderCommitType, error) {
 		"Cannot determine commit type of \"%s\"", commit)
 }
 
-func areChanges(repoDir string) (bool, error) {
-	cmd := []string{
-		"diff",
-		"--name-only",
-	}
-
-	o, err := executeGitCommand(repoDir, cmd, true)
-	if err != nil {
-		return false, err
-	}
-
-	return len(o) > 0, nil
-}
-
 func upstreamFor(path string, commit string) (string, error) {
 	cmd := []string{
 		"rev-parse",
@@ -531,6 +517,70 @@ func (gd *GenericDownloader) cachedFetch(fn func() error) error {
 	return nil
 }
 
+// Indicates whether the specified git repo is in a clean or dirty state.
+//
+// @param path                  The path of the git repo to check.
+//
+// @return string               Text describing repo's dirty state, or "" if
+//                                  clean.
+// @return error                Error.
+func (gd *GenericDownloader) DirtyState(path string) (string, error) {
+	// Check for local changes.
+	cmd := []string{
+		"diff",
+		"--name-only",
+	}
+
+	o, err := executeGitCommand(path, cmd, true)
+	if err != nil {
+		return "", err
+	}
+
+	if len(o) > 0 {
+		return "local changes", nil
+	}
+
+	// Check for staged changes.
+	cmd = []string{
+		"diff",
+		"--name-only",
+		"--staged",
+	}
+
+	o, err = executeGitCommand(path, cmd, true)
+	if err != nil {
+		return "", err
+	}
+
+	if len(o) > 0 {
+		return "staged changes", nil
+	}
+
+	// If on a branch, check for unpushed commits.
+	branch, err := gd.CurrentBranch(path)
+	if err != nil {
+		return "", err
+	}
+
+	if branch != "" {
+		cmd = []string{
+			"rev-list",
+			"@{u}..",
+		}
+
+		o, err = executeGitCommand(path, cmd, true)
+		if err != nil {
+			return "", err
+		}
+
+		if len(o) > 0 {
+			return "unpushed commits", nil
+		}
+	}
+
+	return "", nil
+}
+
 func (gd *GithubDownloader) fetch(repoDir string) error {
 	return gd.cachedFetch(func() error {
 		util.StatusMessage(util.VERBOSITY_VERBOSE, "Fetching repo %s\n",
@@ -593,10 +643,6 @@ func (gd *GithubDownloader) Pull(path string, branchName string) error {
 	return nil
 }
 
-func (gd *GithubDownloader) AreChanges(path string) (bool, error) {
-	return areChanges(path)
-}
-
 func (gd *GithubDownloader) remoteUrls() (string, string) {
 	server := "github.com"
 
@@ -751,10 +797,6 @@ func (gd *GitDownloader) Pull(path string, branchName string) error {
 	return nil
 }
 
-func (gd *GitDownloader) AreChanges(path string) (bool, error) {
-	return areChanges(path)
-}
-
 func (gd *GitDownloader) Clone(commit string, dstPath string) error {
 	// Currently only the master branch is supported.
 	branch := "master"
@@ -831,10 +873,6 @@ func (ld *LocalDownloader) Pull(path string, branchName string) error {
 	return ld.Clone(branchName, path)
 }
 
-func (ld *LocalDownloader) AreChanges(path string) (bool, error) {
-	return areChanges(path)
-}
-
 func (ld *LocalDownloader) Clone(commit string, dstPath string) error {
 	util.StatusMessage(util.VERBOSITY_DEFAULT,
 		"Downloading local repository %s\n", ld.Path)
diff --git a/newt/install/install.go b/newt/install/install.go
index 34c83f72..c5fa0e8d 100644
--- a/newt/install/install.go
+++ b/newt/install/install.go
@@ -188,18 +188,21 @@ func NewInstaller(repos deprepo.RepoMap,
 	}
 
 	// Detect the installed versions of all repos.
+	var firstErr error
 	for n, r := range inst.repos {
 		if !r.IsLocal() && !r.IsNewlyCloned() {
 			ver, err := detectVersion(r)
 			if err != nil {
-				return inst, err
+				if firstErr == nil {
+					firstErr = err
+				}
+			} else {
+				inst.vers[n] = ver
 			}
-
-			inst.vers[n] = ver
 		}
 	}
 
-	return inst, nil
+	return inst, firstErr
 }
 
 // Retrieves the installed version of the specified repo.  Versions get
@@ -535,6 +538,7 @@ func (inst *Installer) installPrompt(vm deprepo.VersionMap, op installOp,
 	}
 
 	for {
+		fmt.Printf("Proceed? [Y/n] ")
 		line, more, err := bufio.NewReader(os.Stdin).ReadLine()
 		if more || err != nil {
 			return false, util.ChildNewtError(err)
@@ -695,6 +699,45 @@ func (inst *Installer) calcVersionMap(candidates []*repo.Repo) (
 	return vm, nil
 }
 
+// Checks if any repos in the specified slice are in a dirty state.  If any
+// repos are dirty and `force` is *not* enabled, an error is returned.  If any
+// repos are dirty and `force` is enabled, a warning is displayed.
+func verifyRepoDirtyState(repos []*repo.Repo, force bool) error {
+	// [repo] => dirty-state.
+	var m map[*repo.Repo]string
+
+	// Collect all dirty repos and insert them into m.
+	for _, r := range repos {
+		dirtyState, err := r.DirtyState()
+		if err != nil {
+			return err
+		}
+
+		if dirtyState != "" {
+			if m == nil {
+				m = make(map[*repo.Repo]string)
+				m[r] = dirtyState
+			}
+		}
+	}
+
+	if len(m) > 0 {
+		s := "some repos are in a dirty state:\n"
+		for r, d := range m {
+			s += fmt.Sprintf("    %s: contains %s\n", r.Name(), d)
+		}
+
+		if !force {
+			s += "Specify the `-f` (force) switch to attempt anyway"
+			return util.NewNewtError(s)
+		} else {
+			util.StatusMessage(util.VERBOSITY_QUIET, "WARNING: %s\n", s)
+		}
+	}
+
+	return nil
+}
+
 // Installs the specified set of repos.
 func (inst *Installer) Install(
 	candidates []*repo.Repo, force bool, ask bool) error {
@@ -759,7 +802,13 @@ func (inst *Installer) Install(
 }
 
 // Installs or upgrades the specified set of repos.
-func (inst *Installer) Upgrade(candidates []*repo.Repo, ask bool) error {
+func (inst *Installer) Upgrade(candidates []*repo.Repo, force bool,
+	ask bool) error {
+
+	if err := verifyRepoDirtyState(candidates, force); err != nil {
+		return err
+	}
+
 	vm, err := inst.calcVersionMap(candidates)
 	if err != nil {
 		return err
@@ -801,7 +850,13 @@ func (inst *Installer) Upgrade(candidates []*repo.Repo, ask bool) error {
 }
 
 // Syncs the specified set of repos.
-func (inst *Installer) Sync(candidates []*repo.Repo, ask bool) error {
+func (inst *Installer) Sync(candidates []*repo.Repo,
+	force bool, ask bool) error {
+
+	if err := verifyRepoDirtyState(candidates, force); err != nil {
+		return err
+	}
+
 	vm, err := inst.calcVersionMap(candidates)
 	if err != nil {
 		return err
@@ -846,3 +901,89 @@ func (inst *Installer) Sync(candidates []*repo.Repo, ask bool) error {
 
 	return nil
 }
+
+type repoInfo struct {
+	installedVer *newtutil.RepoVersion
+	errorText    string
+	dirtyState   string
+	needsUpgrade bool
+}
+
+// Collects information about the specified repo.  If a version map is provided
+// (i.e., vm is not nil), this function also queries the repo's remote to
+// determine if the repo can be upgraded.
+func (inst *Installer) gatherInfo(r *repo.Repo,
+	vm *deprepo.VersionMap) repoInfo {
+
+	ri := repoInfo{}
+
+	if !r.CheckExists() {
+		return ri
+	}
+
+	ver, err := r.InstalledVersion()
+	if err != nil {
+		ri.errorText = strings.TrimSpace(err.Error())
+		return ri
+	}
+	ri.installedVer = ver
+
+	dirty, err := r.DirtyState()
+	if err != nil {
+		ri.errorText = strings.TrimSpace(err.Error())
+		return ri
+	}
+	ri.dirtyState = dirty
+
+	if vm != nil {
+		if ver == nil || *ver != (*vm)[r.Name()] {
+			ri.needsUpgrade = true
+		}
+	}
+
+	return ri
+}
+
+// Prints out information about the specified repos:
+//     * Currently installed version.
+//     * Whether upgrade is possible.
+//     * Whether repo is in a dirty state.
+//
+// @param repos                 The set of repositories to inspect.
+// @param remote                Whether to perform any remote queries to
+//                                  determine if upgrades are needed.
+func (inst *Installer) Info(repos []*repo.Repo, remote bool) error {
+	var vmp *deprepo.VersionMap
+
+	if remote {
+		vm, err := inst.calcVersionMap(repos)
+		if err != nil {
+			return err
+		}
+
+		vmp = &vm
+	}
+
+	util.StatusMessage(util.VERBOSITY_DEFAULT, "Repository info:\n")
+	for _, r := range repos {
+		ri := inst.gatherInfo(r, vmp)
+		s := fmt.Sprintf("    * %s:", r.Name())
+
+		if ri.installedVer == nil {
+			s += " (not installed)"
+		} else if ri.errorText != "" {
+			s += fmt.Sprintf(" (unknown: %s)", ri.errorText)
+		} else {
+			s += fmt.Sprintf(" %s", ri.installedVer.String())
+			if ri.dirtyState != "" {
+				s += fmt.Sprintf(" (dirty: %s)", ri.dirtyState)
+			}
+			if ri.needsUpgrade {
+				s += " (needs upgrade)"
+			}
+		}
+		util.StatusMessage(util.VERBOSITY_DEFAULT, "%s\n", s)
+	}
+
+	return nil
+}
diff --git a/newt/project/project.go b/newt/project/project.go
index 7289abfb..fb29d374 100644
--- a/newt/project/project.go
+++ b/newt/project/project.go
@@ -279,7 +279,7 @@ func (proj *Project) InstallIf(
 	}
 
 	if upgrade {
-		return inst.Upgrade(specifiedRepoList, ask)
+		return inst.Upgrade(specifiedRepoList, force, ask)
 	} else {
 		return inst.Install(specifiedRepoList, force, ask)
 	}
@@ -302,7 +302,30 @@ func (proj *Project) SyncIf(
 		return err
 	}
 
-	return inst.Sync(repoList, ask)
+	return inst.Sync(repoList, force, ask)
+}
+
+func (proj *Project) InfoIf(predicate func(r *repo.Repo) bool,
+	remote bool) error {
+
+	if remote {
+		// Make sure we have an up to date copy of all `repository.yml` files.
+		if err := proj.downloadRepositoryYmlFiles(); err != nil {
+			return err
+		}
+	}
+
+	// Determine which repos the user wants info about.
+	repoList := proj.SelectRepos(predicate)
+
+	// Ignore errors.  We will deal with bad repos individually when we display
+	// info about them.
+	inst, _ := install.NewInstaller(proj.repos, proj.rootRepoReqs)
+	if err := inst.Info(repoList, remote); err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // Loads a complete repo definition from the appropriate `repository.yml` file.
diff --git a/newt/repo/repo.go b/newt/repo/repo.go
index 992e10a4..da381e7c 100644
--- a/newt/repo/repo.go
+++ b/newt/repo/repo.go
@@ -217,7 +217,7 @@ func (r *Repo) downloadRepo(commit string) error {
 	return nil
 }
 
-func (r *Repo) checkExists() bool {
+func (r *Repo) CheckExists() bool {
 	return util.NodeExist(r.Path())
 }
 
@@ -236,6 +236,15 @@ func (r *Repo) updateRepo(commit string) error {
 	return nil
 }
 
+// Indicates whether the specified repo is in a clean or dirty state.
+//
+// @return string               Text describing repo's dirty state, or "" if
+//                                  clean.
+// @return error                Error.
+func (r *Repo) DirtyState() (string, error) {
+	return r.downloader.DirtyState(r.Path())
+}
+
 func (r *Repo) Install(ver newtutil.RepoVersion) error {
 	commit, err := r.CommitFromVer(ver)
 	if err != nil {
@@ -255,17 +264,6 @@ func (r *Repo) Upgrade(ver newtutil.RepoVersion) error {
 		return err
 	}
 
-	changes, err := r.downloader.AreChanges(r.Path())
-	if err != nil {
-		return err
-	}
-
-	if changes {
-		return util.FmtNewtError(
-			"Repository \"%s\" contains local changes.  Please resolve "+
-				"changes manually and try again.", r.Name())
-	}
-
 	if err := r.updateRepo(commit); err != nil {
 		return err
 	}
@@ -353,7 +351,7 @@ func (r *Repo) UpdateDesc() (bool, error) {
 
 func (r *Repo) ensureExists() error {
 	// Clone the repo if it doesn't exist.
-	if util.NodeNotExist(r.localPath) {
+	if !r.CheckExists() {
 		if err := r.downloadRepo("master"); err != nil {
 			return err
 		}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services