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/08/29 22:22:07 UTC
[GitHub] ccollins476ad closed pull request #204: Don't start an upgrade or
sync if repos are dirty
ccollins476ad closed pull request #204: Don't start an upgrade or sync if repos are dirty
URL: https://github.com/apache/mynewt-newt/pull/204
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..25bb6998 100644
--- a/newt/cli/project_cmds.go
+++ b/newt/cli/project_cmds.go
@@ -91,7 +91,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 +125,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); 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 +146,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 {
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..5625ad33 100644
--- a/newt/install/install.go
+++ b/newt/install/install.go
@@ -535,6 +535,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 +696,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 +799,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 +847,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 +898,38 @@ func (inst *Installer) Sync(candidates []*repo.Repo, ask bool) error {
return nil
}
+
+// Prints out information about the specified repos:
+// * Currently installed version.
+// * Whether upgrade is possible.
+// * Whether repo is in a dirty state.
+func (inst *Installer) Info(repos []*repo.Repo) error {
+ vm, err := inst.calcVersionMap(repos)
+ if err != nil {
+ return err
+ }
+
+ util.StatusMessage(util.VERBOSITY_DEFAULT, "Repository info:\n")
+ for _, r := range repos {
+ ver, err := r.InstalledVersion()
+ if err != nil {
+ return err
+ }
+
+ dirty, err := r.DirtyState()
+ if err != nil {
+ return err
+ }
+
+ s := fmt.Sprintf(" * %s: %s", r.Name(), ver.String())
+ if ver == nil || *ver != vm[r.Name()] {
+ s += " (needs upgrade)"
+ }
+ if dirty != "" {
+ s += fmt.Sprintf(" (dirty: %s)", dirty)
+ }
+ util.StatusMessage(util.VERBOSITY_DEFAULT, "%s\n", s)
+ }
+
+ return nil
+}
diff --git a/newt/project/project.go b/newt/project/project.go
index 7289abfb..a6ba960f 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,28 @@ 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) error {
+ // 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)
+
+ inst, err := install.NewInstaller(proj.repos, proj.rootRepoReqs)
+ if err != nil {
+ return err
+ }
+
+ if err := inst.Info(repoList); 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..ba1a9349 100644
--- a/newt/repo/repo.go
+++ b/newt/repo/repo.go
@@ -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
}
----------------------------------------------------------------
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