This is an automated email from the ASF dual-hosted git repository.

ccollins pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git


The following commit(s) were added to refs/heads/master by this push:
     new 6a51e35  Use "detached head" instead of local branches
6a51e35 is described below

commit 6a51e35565323ebe8feb8d1aa6e00960b6ce662e
Author: Christopher Collins <ccoll...@apache.org>
AuthorDate: Thu Jul 18 13:23:00 2019 -0700

    Use "detached head" instead of local branches
    
    SUMMARY:
    
    This commit changes how newt interacts with git.  Newt used to use a mix
    of "detached head" and local branches.  With this commit, newt uses
    "detached head" exclusively.
    
    OLD BEHAVIOR:
    
    Newt used a combination of "detached head" and local branches.
    
    When upgrading to a version mapped to a branch, newt would
    check out a local branch with the appropriate name.  For example,
    when upgrading to 0.0.0, newt would check out a local "master" branch.
    
    When upgrading to a version mapped to a tag or a commit hash, newt would
    put the repo in a "detached head" state.
    
    This inconsistency comes from the `git checkout` command.  Git puts a
    repo in a different state depending on the type of commit being checked
    out.
    
    NEW BEHAVIOR:
    
    Newt uses "detached head" exclusively.  When upgrading to a version
    mapped to a branch or tag, newt looks up the corresponding commit hash
    and checks it out.
    
    In addition, this commit adds a clarification in semantics:
    
        Only the following commands make modifications to git repos:
            * new
            * install
            * upgrade
            * info -r
    
        All other commands leave the git repos untouched.  E.g., they do not
        run `git fetch`, `git checkout`, or `git remote set-url`.
    
    This change simplifies working with custom branches.
    
    IMPROVEMENTS:
    
    This change improves newt in several ways:
    
    1. Lower chance of git conflicts.  When newt used local branches, there
    was always a chance of pulling incompatible commits into an existing
    branch.  This was especially likely when using `newt sync` and changing
    a repo's remote URL.
    
    2. No need for the `newt sync` command.  The sync command was only
    necessary due to newt's use of local branches.  Now, `newt upgrade`
    alone does the right thing: it fetches from `origin` and checks out the
    correct commit.  For branch-backed versions, newt checks out the latest
    commit in the branch.
    
    The `newt sync` command has been deprecated.  Now it prints a warning
    message and performs a `newt upgrade`.
    
    3. Proper support for branches and tags in `project.yml`.  Newt has
    always allowed the `<hash>-commit` notation for a repo version string,
    but it did not correctly handle `<branch>-commit` or `<tag>-commit`.
    Now these version strings work correctly.
---
 newt/cli/project_cmds.go      |   8 +-
 newt/downloader/downloader.go | 516 +++++++++++++++++++++---------------------
 newt/install/install.go       | 106 +++------
 newt/project/project.go       |  39 ++--
 newt/repo/repo.go             |  90 +++-----
 newt/repo/version.go          |  22 +-
 6 files changed, 344 insertions(+), 437 deletions(-)

diff --git a/newt/cli/project_cmds.go b/newt/cli/project_cmds.go
index 4204524..9344e0f 100644
--- a/newt/cli/project_cmds.go
+++ b/newt/cli/project_cmds.go
@@ -183,8 +183,10 @@ func syncRunCmd(cmd *cobra.Command, args []string) {
        proj := TryGetProject()
        pred := makeRepoPredicate(args)
 
-       if err := proj.SyncIf(
-               newtutil.NewtForce, newtutil.NewtAsk, pred); err != nil {
+       util.OneTimeWarning("\"sync\" is deprecated.  Use \"upgrade\" instead.")
+
+       if err := proj.InstallIf(
+               true, newtutil.NewtForce, newtutil.NewtAsk, pred); err != nil {
 
                NewtUsage(nil, err)
        }
@@ -239,7 +241,7 @@ func AddProjectCommands(cmd *cobra.Command) {
        syncHelpEx += "    Syncs the apache-mynewt-core repository."
        syncCmd := &cobra.Command{
                Use:     "sync [repo-1] [repo-2] [...]",
-               Short:   "Synchronize project dependencies",
+               Short:   "Synchronize project dependencies (deprecated)",
                Long:    syncHelpText,
                Example: syncHelpEx,
                Run:     syncRunCmd,
diff --git a/newt/downloader/downloader.go b/newt/downloader/downloader.go
index 99a21d8..1e58ea4 100644
--- a/newt/downloader/downloader.go
+++ b/newt/downloader/downloader.go
@@ -56,8 +56,12 @@ type Downloader interface {
        // (i.e., 1 hash, n tags, and n branches).
        CommitsFor(path string, commit string) ([]string, error)
 
-       // Fetches all remotes and merges the specified branch into the local 
repo.
-       Pull(path string, branchName string) error
+       // Fetches all remotes.
+       Fetch(path string) error
+
+       // Checks out the specified commit (hash, tag, or branch).  Always puts 
the
+       // repo in a "detached head" state.
+       Checkout(path string, commit string) error
 
        // Indicates whether the repo is in a clean or dirty state.
        DirtyState(path string) (string, error)
@@ -69,16 +73,24 @@ type Downloader interface {
        // user's `project.yml` file and / or the repo dependency lists.
        FixupOrigin(path string) error
 
-       // Retrieves the name of the currently checked out branch, or "" if no
-       // branch is checked out.
+       // Retrieves the name of the currently checked out local branch, or "" 
if
+       // the repo is in a "detached head" state.
        CurrentBranch(path string) (string, error)
+}
 
-       // Retrieves the name of the remote branch being tracked by the 
specified
-       // local branch, or "" if there is no tracked remote branch.
-       UpstreamFor(repoDir string, branch string) (string, error)
+type Commit struct {
+       hash string
+       name string
+       typ  DownloaderCommitType
 }
 
 type GenericDownloader struct {
+       // [name-of-branch-or-tag]commit
+       commits map[string]Commit
+
+       // Hash of checked-out commit.
+       head string
+
        // Whether 'origin' has been fetched during this run.
        fetched bool
 }
@@ -188,80 +200,12 @@ func updateSubmodules(path string) error {
        return nil
 }
 
-// checkout does checkout a branch, or create a new branch from a tag name
-// if the commit supplied is a tag. sha1 based commits have no special
-// handling and result in dettached from HEAD state.
-func checkout(repoDir string, commit string) error {
-       var cmd []string
-       ct, err := commitType(repoDir, commit)
-       if err != nil {
-               return err
-       }
-
-       full, err := remoteCommitName(repoDir, commit)
-       if err != nil {
-               return err
-       }
-
-       if ct == COMMIT_TYPE_TAG {
-               util.StatusMessage(util.VERBOSITY_VERBOSE, "Will create new 
branch %s"+
-                       " from %s\n", commit, full)
-               cmd = []string{
-                       "checkout",
-                       full,
-                       "-b",
-                       commit,
-               }
-       } else {
-               util.StatusMessage(util.VERBOSITY_VERBOSE, "Will checkout 
%s\n", full)
-               cmd = []string{
-                       "checkout",
-                       commit,
-               }
-       }
-       if _, err := executeGitCommand(repoDir, cmd, true); err != nil {
-               return err
-       }
-
-       // Always initialize and update submodules on checkout.  This prevents 
the
-       // repo from being in a modified "(new commits)" state immediately after
-       // switching commits.  If the submodules have already been updated, this
-       // does not generate any network activity.
-       if err := initSubmodules(repoDir); err != nil {
-               return err
-       }
-       if err := updateSubmodules(repoDir); err != nil {
-               return err
-       }
-
-       return nil
-}
-
-// rebase applies upstream changes to the local copy and must be
-// preceeded by a "fetch" to achieve any meaningful result.
-func rebase(repoDir string, commit string) error {
-       if err := checkout(repoDir, commit); err != nil {
-               return err
-       }
-
-       // We want to rebase the remote version of this branch.
-       full, err := remoteCommitName(repoDir, commit)
-       if err != nil {
-               return err
-       }
-
-       cmd := []string{
-               "rebase",
-               full}
-       if _, err := executeGitCommand(repoDir, cmd, true); err != nil {
-               util.StatusMessage(util.VERBOSITY_VERBOSE,
-                       "Merging changes from %s: %s\n", full, err)
-               return err
-       }
-
-       util.StatusMessage(util.VERBOSITY_VERBOSE,
-               "Merging changes from %s\n", full)
-       return nil
+// fixupCommitString strips "origin/" from the front of a commit, if it is
+// present.  Newt only works with remote branches, and only with the "origin"
+// remote.  The user is not required to prefix his branch specifiers with
+// "origin/", but is allowed to.
+func fixupCommitString(s string) string {
+       return strings.TrimPrefix(s, "origin/")
 }
 
 func mergeBase(repoDir string, commit string) (string, error) {
@@ -278,39 +222,6 @@ func mergeBase(repoDir string, commit string) (string, 
error) {
        return strings.TrimSpace(string(o)), nil
 }
 
-func branchExists(repoDir string, branchName string) bool {
-       cmd := []string{
-               "show-ref",
-               "--verify",
-               "--quiet",
-               "refs/heads/" + branchName,
-       }
-       _, err := executeGitCommand(repoDir, cmd, true)
-       return err == nil
-}
-
-func commitType(repoDir string, commit string) (DownloaderCommitType, error) {
-       if commit == "HEAD" {
-               return COMMIT_TYPE_HASH, nil
-       }
-
-       if _, err := mergeBase(repoDir, commit); err == nil {
-               // Distinguish local branch from hash.
-               if branchExists(repoDir, commit) {
-                       return COMMIT_TYPE_BRANCH, nil
-               } else {
-                       return COMMIT_TYPE_HASH, nil
-               }
-       }
-
-       if _, err := mergeBase(repoDir, "tags/"+commit); err == nil {
-               return COMMIT_TYPE_TAG, nil
-       }
-
-       return DownloaderCommitType(-1), util.FmtNewtError(
-               "Cannot determine commit type of \"%s\"", commit)
-}
-
 func upstreamFor(path string, commit string) (string, error) {
        cmd := []string{
                "rev-parse",
@@ -331,64 +242,6 @@ func upstreamFor(path string, commit string) (string, 
error) {
        return strings.TrimSpace(string(up)), nil
 }
 
-func remoteCommitName(path string, commit string) (string, error) {
-       ct, err := commitType(path, commit)
-       if err != nil {
-               return "", err
-       }
-
-       switch ct {
-       case COMMIT_TYPE_BRANCH:
-               rmt, err := upstreamFor(path, commit)
-               if err != nil {
-                       return "", err
-               }
-               if rmt == "" {
-                       return "",
-                               util.FmtNewtError("No remote upstream for 
branch \"%s\"",
-                                       commit)
-               }
-               return rmt, nil
-       case COMMIT_TYPE_TAG:
-               return "tags/" + commit, nil
-       case COMMIT_TYPE_HASH:
-               return commit, nil
-       default:
-               return "", util.FmtNewtError("unknown commit type: %d", int(ct))
-       }
-}
-
-func showFile(
-       path string, branch string, filename string, dstDir string) error {
-
-       if err := os.MkdirAll(dstDir, os.ModePerm); err != nil {
-               return util.ChildNewtError(err)
-       }
-
-       full, err := remoteCommitName(path, branch)
-       if err != nil {
-               return err
-       }
-
-       cmd := []string{
-               "show",
-               fmt.Sprintf("%s:%s", full, filename),
-       }
-
-       dstPath := fmt.Sprintf("%s/%s", dstDir, filename)
-       log.Debugf("Fetching file %s to %s", filename, dstPath)
-       data, err := executeGitCommand(path, cmd, true)
-       if err != nil {
-               return err
-       }
-
-       if err := ioutil.WriteFile(dstPath, data, os.ModePerm); err != nil {
-               return util.ChildNewtError(err)
-       }
-
-       return nil
-}
-
 func getRemoteUrl(path string, remote string) (string, error) {
        cmd := []string{
                "remote",
@@ -426,80 +279,249 @@ func warnWrongOriginUrl(path string, curUrl string, 
goodUrl string) {
                curUrl, goodUrl)
 }
 
-func (gd *GenericDownloader) CommitType(
-       path string, commit string) (DownloaderCommitType, error) {
+// getCommits gathers all tags and remote branches.  It returns a mapping of
+// [name]commit.
+func getCommits(path string) (map[string]Commit, error) {
+       cmd := []string{"show-ref", "--dereference"}
+       o, err := executeGitCommand(path, cmd, true)
+       if err != nil {
+               return nil, err
+       }
+
+       // Example output:
+       // b7a5474d569d5b67152d1773627ddda010c080a3 
refs/remotes/origin/1_7_0_dev
+       // da13fb50c3b5824c47a44b62c3c9f693b922ce9c refs/tags/mynewt_1_7_0_tag
+       // b7a5474d569d5b67152d1773627ddda010c080a3 
refs/tags/mynewt_1_7_0_tag^{}
+
+       m := map[string]Commit{}
 
-       return commitType(path, commit)
+       lines := strings.Split(strings.TrimSpace(string(o)), "\n")
+       for _, line := range lines {
+               f := strings.Fields(line)
+               if len(f) != 2 {
+                       return nil, util.FmtNewtError(
+                               "git show-ref produced unexpected line: 
\"%s\"", line)
+               }
+
+               hash := f[0]
+               ref := strings.TrimSuffix(f[1], "^{}")
+
+               c := Commit{
+                       hash: hash,
+               }
+               if n := strings.TrimPrefix(ref, "refs/remotes/origin/"); n != 
ref {
+                       c.typ = COMMIT_TYPE_BRANCH
+                       c.name = n
+               } else if n := strings.TrimPrefix(ref, "refs/tags/"); n != ref {
+                       c.typ = COMMIT_TYPE_TAG
+                       c.name = n
+               }
+
+               if c.name != "" {
+                       m[c.name] = c
+               }
+       }
+
+       return m, nil
 }
 
-func (gd *GenericDownloader) HashFor(path string, commit string) (string, 
error) {
-       full, err := remoteCommitName(path, commit)
+// init populates a generic downloader with branch and tag information.
+func (gd *GenericDownloader) init(path string) error {
+       cmap, err := getCommits(path)
        if err != nil {
-               return "", err
+               return err
        }
-       cmd := []string{"rev-parse", full}
+       gd.commits = cmap
+
+       cmd := []string{"rev-parse", "HEAD"}
        o, err := executeGitCommand(path, cmd, true)
        if err != nil {
-               return "", err
+               return err
        }
+       gd.head = strings.TrimSpace(string(o))
 
-       return strings.TrimSpace(string(o)), nil
+       return nil
 }
 
-func (gd *GenericDownloader) CommitsFor(
-       path string, commit string) ([]string, error) {
+// ensureInited calls init on the provided downloader if it has not already
+// been initialized.
+func (gd *GenericDownloader) ensureInited(path string) error {
+       if gd.commits != nil {
+               // Already initialized.
+               return nil
+       }
 
-       // Hash.
-       hash, err := gd.HashFor(path, commit)
+       return gd.init(path)
+}
+
+func (gd *GenericDownloader) Checkout(repoDir string, commit string) error {
+       // Get the hash corresponding to the commit in case the caller 
specified a
+       // branch or tag.  We always want to check out a hash and end up in a
+       // "detached head" state.
+       hash, err := gd.HashFor(repoDir, commit)
        if err != nil {
-               return nil, err
+               return err
        }
 
-       // Branches and tags.
+       util.StatusMessage(util.VERBOSITY_VERBOSE, "Will checkout %s\n", hash)
        cmd := []string{
-               "for-each-ref",
-               "--format=%(refname:short)",
-               "--points-at",
+               "checkout",
                hash,
        }
-       o, err := executeGitCommand(path, cmd, true)
+
+       if _, err := executeGitCommand(repoDir, cmd, true); err != nil {
+               return err
+       }
+
+       // Always initialize and update submodules on checkout.  This prevents 
the
+       // repo from being in a modified "(new commits)" state immediately after
+       // switching commits.  If the submodules have already been updated, this
+       // does not generate any network activity.
+       if err := initSubmodules(repoDir); err != nil {
+               return err
+       }
+       if err := updateSubmodules(repoDir); err != nil {
+               return err
+       }
+
+       return nil
+}
+
+func (gd *GenericDownloader) showFile(
+       path string, commit string, filename string, dstDir string) error {
+
+       if err := os.MkdirAll(dstDir, os.ModePerm); err != nil {
+               return util.ChildNewtError(err)
+       }
+
+       hash, err := gd.HashFor(path, commit)
        if err != nil {
+               return err
+       }
+
+       dstPath := fmt.Sprintf("%s/%s", dstDir, filename)
+       log.Debugf("Fetching file %s to %s", filename, dstPath)
+
+       cmd := []string{"show", fmt.Sprintf("%s:%s", hash, filename)}
+       data, err := executeGitCommand(path, cmd, true)
+       if err != nil {
+               return err
+       }
+
+       if err := ioutil.WriteFile(dstPath, data, os.ModePerm); err != nil {
+               return util.ChildNewtError(err)
+       }
+
+       return nil
+}
+
+func (gd *GenericDownloader) findCommit(s string) *Commit {
+       c, ok := gd.commits[fixupCommitString(s)]
+       if !ok {
+               return nil
+       } else {
+               return &c
+       }
+}
+
+func (gd *GenericDownloader) CommitType(
+       path string, commit string) (DownloaderCommitType, error) {
+
+       if err := gd.ensureInited(path); err != nil {
+               return -1, err
+       }
+
+       // HEAD is always a commit hash (detached).
+       if commit == "HEAD" {
+               return COMMIT_TYPE_HASH, nil
+       }
+
+       // Check if user provided a branch or tag name.
+       if c := gd.findCommit(commit); c != nil {
+               return c.typ, nil
+       }
+
+       // Check if user provided a commit hash.
+       if _, err := mergeBase(path, commit); err == nil {
+               return COMMIT_TYPE_HASH, nil
+       }
+
+       return -1, util.FmtNewtError(
+               "cannot determine commit type of \"%s\"", commit)
+}
+
+func (gd *GenericDownloader) HashFor(path string,
+       commit string) (string, error) {
+
+       if err := gd.ensureInited(path); err != nil {
+               return "", err
+       }
+
+       if commit == "HEAD" {
+               return gd.head, nil
+       }
+
+       if c := gd.findCommit(commit); c != nil {
+               return c.hash, nil
+       }
+
+       return commit, nil
+}
+
+func (gd *GenericDownloader) CommitsFor(
+       path string, commit string) ([]string, error) {
+
+       if err := gd.ensureInited(path); err != nil {
                return nil, err
        }
 
-       lines := []string{hash}
-       text := strings.TrimSpace(string(o))
-       if text != "" {
-               lines = append(lines, strings.Split(text, "\n")...)
+       commit = fixupCommitString(commit)
+
+       var commits []string
+
+       // Always insert the specified string into the set.
+       commits = append(commits, commit)
+
+       // Add all commits that are equivalent to the specified string.
+       for _, c := range gd.commits {
+               if commit == c.hash {
+                       // User specified a hash; add the corresponding branch 
or tag name.
+                       commits = append(commits, c.name)
+               } else if commit == c.name {
+                       // User specified a branch or tag; add the 
corresponding hash.
+                       commits = append(commits, c.hash)
+               }
        }
 
-       sort.Strings(lines)
-       return lines, nil
+       sort.Strings(commits)
+       return commits, nil
 }
 
 func (gd *GenericDownloader) CurrentBranch(path string) (string, error) {
-       cmd := []string{
-               "rev-parse",
-               "--abbrev-ref",
-               "HEAD",
-       }
+       // Check if there is a git ref (branch) for the current commit.  If 
there
+       // is none, git exits with a status of 1.  We need to distinguish this 
case
+       // from an actual error.
+       cmd := []string{"symbolic-ref", "-q", "HEAD"}
        o, err := executeGitCommand(path, cmd, true)
        if err != nil {
-               return "", err
+               ne := err.(*util.NewtError)
+               ee, ok := ne.Parent.(*exec.ExitError)
+               if ok && ee.ExitCode() == 1 {
+                       // No branch.
+                       return "", nil
+               } else {
+                       return "", err
+               }
        }
 
        s := strings.TrimSpace(string(o))
-       if s == "HEAD" {
-               return "", nil
-       } else {
-               return s, nil
+       branch := strings.TrimPrefix(s, "refs/heads/")
+       if branch == s {
+               return "", util.FmtNewtError(
+                       "%s produced unexpected output: %s", strings.Join(cmd, 
" "), s)
        }
-}
 
-func (gd *GenericDownloader) UpstreamFor(repoDir string,
-       branch string) (string, error) {
-
-       return upstreamFor(repoDir, branch)
+       return branch, nil
 }
 
 // Fetches the downloader's origin remote if it hasn't been fetched yet during
@@ -586,12 +608,13 @@ func (gd *GenericDownloader) DirtyState(path string) 
(string, error) {
        return "", nil
 }
 
-func (gd *GithubDownloader) fetch(repoDir string) error {
+func (gd *GithubDownloader) Fetch(repoDir string) error {
        return gd.cachedFetch(func() error {
                util.StatusMessage(util.VERBOSITY_VERBOSE, "Fetching repo %s\n",
                        gd.Repo)
 
-               _, err := gd.authenticatedCommand(repoDir, []string{"fetch", 
"--tags"})
+               cmd := []string{"fetch", "--tags"}
+               _, err := gd.authenticatedCommand(repoDir, cmd)
                return err
        })
 }
@@ -620,28 +643,11 @@ func (gd *GithubDownloader) authenticatedCommand(path 
string,
 func (gd *GithubDownloader) FetchFile(
        commit string, path string, filename string, dstDir string) error {
 
-       if err := gd.fetch(path); err != nil {
-               return err
-       }
-
-       if err := showFile(path, commit, filename, dstDir); err != nil {
+       if err := gd.Fetch(path); err != nil {
                return err
        }
 
-       return nil
-}
-
-func (gd *GithubDownloader) Pull(path string, branchName string) error {
-       err := gd.fetch(path)
-       if err != nil {
-               return err
-       }
-
-       // Ignore error, probably resulting from a branch not available at 
origin
-       // anymore.
-       rebase(path, branchName)
-
-       if err := checkout(path, branchName); err != nil {
+       if err := gd.showFile(path, commit, filename, dstDir); err != nil {
                return err
        }
 
@@ -731,11 +737,9 @@ func (gd *GithubDownloader) Clone(commit string, dstPath 
string) error {
        if err != nil {
                return err
        }
-
        defer gd.clearRemoteAuth(dstPath)
 
-       // Checkout the specified commit.
-       if err := checkout(dstPath, commit); err != nil {
+       if err := gd.Checkout(dstPath, commit); err != nil {
                return err
        }
 
@@ -762,10 +766,8 @@ func NewGithubDownloader() *GithubDownloader {
        return &GithubDownloader{}
 }
 
-func (gd *GitDownloader) fetch(repoDir string) error {
+func (gd *GitDownloader) Fetch(repoDir string) error {
        return gd.cachedFetch(func() error {
-               util.StatusMessage(util.VERBOSITY_VERBOSE, "Fetching repo %s\n",
-                       gd.Url)
                _, err := executeGitCommand(repoDir, []string{"fetch", 
"--tags"}, true)
                return err
        })
@@ -774,28 +776,11 @@ func (gd *GitDownloader) fetch(repoDir string) error {
 func (gd *GitDownloader) FetchFile(
        commit string, path string, filename string, dstDir string) error {
 
-       if err := gd.fetch(path); err != nil {
-               return err
-       }
-
-       if err := showFile(path, commit, filename, dstDir); err != nil {
-               return err
-       }
-
-       return nil
-}
-
-func (gd *GitDownloader) Pull(path string, branchName string) error {
-       err := gd.fetch(path)
-       if err != nil {
+       if err := gd.Fetch(path); err != nil {
                return err
        }
 
-       // Ignore error, probably resulting from a branch not available at 
origin
-       // anymore.
-       rebase(path, branchName)
-
-       if err := checkout(path, branchName); err != nil {
+       if err := gd.showFile(path, commit, filename, dstDir); err != nil {
                return err
        }
 
@@ -833,8 +818,7 @@ func (gd *GitDownloader) Clone(commit string, dstPath 
string) error {
                return err
        }
 
-       // Checkout the specified commit.
-       if err := checkout(dstPath, commit); err != nil {
+       if err := gd.Checkout(dstPath, commit); err != nil {
                return err
        }
 
@@ -873,9 +857,14 @@ func (ld *LocalDownloader) FetchFile(
        return nil
 }
 
-func (ld *LocalDownloader) Pull(path string, branchName string) error {
+func (ld *LocalDownloader) Fetch(path string) error {
        os.RemoveAll(path)
-       return ld.Clone(branchName, path)
+       return ld.Clone("master", path)
+}
+
+func (ld *LocalDownloader) Checkout(path string, commit string) error {
+       _, err := executeGitCommand(path, []string{"checkout", commit}, true)
+       return err
 }
 
 func (ld *LocalDownloader) Clone(commit string, dstPath string) error {
@@ -886,8 +875,7 @@ func (ld *LocalDownloader) Clone(commit string, dstPath 
string) error {
                return err
        }
 
-       // Checkout the specified commit.
-       if err := checkout(dstPath, commit); err != nil {
+       if err := ld.Checkout(dstPath, commit); err != nil {
                return err
        }
 
diff --git a/newt/install/install.go b/newt/install/install.go
index 0c08032..5244999 100644
--- a/newt/install/install.go
+++ b/newt/install/install.go
@@ -133,7 +133,6 @@ type installOp int
 const (
        INSTALL_OP_INSTALL installOp = iota
        INSTALL_OP_UPGRADE
-       INSTALL_OP_SYNC
 )
 
 // Determines the currently installed version of the specified repo.  If the
@@ -408,16 +407,20 @@ func (inst *Installer) shouldUpgradeRepo(
                        "internal error: nonexistent repo has version: %s", 
repoName)
        }
 
-       if !r.VersionsEqual(*curVer, destVer) {
+       // If the repo is not in a "detached head" state, it needs to be fixed 
up.
+       detached, err := r.IsDetached()
+       if err != nil {
+               return false, err
+       }
+       if !detached {
                return true, nil
        }
 
-       equiv, err := r.CommitsEquivalent(curVer.Commit, destVer.Commit)
-       if err != nil {
-               return false, err
+       if !r.VersionsEqual(*curVer, destVer) {
+               return true, nil
        }
 
-       return !equiv, nil
+       return false, nil
 }
 
 // Removes repos that shouldn't be upgraded from the specified list.  A repo
@@ -460,7 +463,7 @@ func (inst *Installer) filterUpgradeList(
 // Describes an imminent install or upgrade operation to the user.  The
 // displayed message applies to the specified repo.
 func (inst *Installer) installMessageOneRepo(
-       repoName string, op installOp, force bool, curVer *newtutil.RepoVersion,
+       r *repo.Repo, op installOp, force bool, curVer *newtutil.RepoVersion,
        destVer newtutil.RepoVersion) (string, error) {
 
        // If the repo isn't installed yet, this is an install, not an upgrade.
@@ -478,24 +481,22 @@ func (inst *Installer) installMessageOneRepo(
                }
 
        case INSTALL_OP_UPGRADE:
-               verb = "upgrade"
-
-       case INSTALL_OP_SYNC:
-               verb = "sync"
+               if r.VersionsEqual(*curVer, destVer) {
+                       verb = "fixup"
+               } else {
+                       verb = "upgrade"
+               }
 
        default:
                return "", util.FmtNewtError(
                        "internal error: invalid install op: %v", op)
        }
 
-       msg := fmt.Sprintf("    %s %s ", verb, repoName)
-       if op == INSTALL_OP_UPGRADE {
+       msg := fmt.Sprintf("    %s %s ", verb, r.Name())
+       if verb == "upgrade" {
                msg += fmt.Sprintf("(%s --> %s)", curVer.String(), 
destVer.String())
-       } else if op != INSTALL_OP_SYNC {
-               msg += fmt.Sprintf("(%s)", destVer.String())
        } else {
-               // Sync operation.  Don't print the project version.  Instead, 
print
-               // the actual branch name later during the sync.
+               msg += fmt.Sprintf("(%s)", destVer.String())
        }
 
        return msg, nil
@@ -526,7 +527,7 @@ func (inst *Installer) installPrompt(vm deprepo.VersionMap, 
op installOp,
                destVer := vm[name]
 
                msg, err := inst.installMessageOneRepo(
-                       name, op, force, curVer, destVer)
+                       r, op, force, curVer, destVer)
                if err != nil {
                        return false, err
                }
@@ -880,59 +881,6 @@ func (inst *Installer) Upgrade(candidates []*repo.Repo, 
force bool,
        return nil
 }
 
-// Syncs the specified set of repos.
-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
-       }
-
-       // Notify the user of what install operations are about to happen, and
-       // prompt if the `-a` (ask) option was specified.
-       proceed, err := inst.installPrompt(vm, INSTALL_OP_SYNC, false, ask)
-       if err != nil {
-               return err
-       }
-       if !proceed {
-               return nil
-       }
-
-       repos, err := inst.versionMapRepos(vm)
-       if err != nil {
-               return err
-       }
-
-       // Sync each repo in the list.
-       var anyFails bool
-       for _, r := range repos {
-               ver := inst.installedVer(r.Name())
-               if ver == nil {
-                       util.StatusMessage(util.VERBOSITY_DEFAULT,
-                               "No installed version of %s found, skipping\n",
-                               r.Name())
-               } else {
-                       if _, err := r.Sync(*ver); err != nil {
-                               util.StatusMessage(util.VERBOSITY_QUIET,
-                                       "Failed to sync repo \"%s\": %s\n",
-                                       r.Name(), err.Error())
-                               anyFails = true
-                       }
-               }
-       }
-
-       if anyFails {
-               return util.FmtNewtError("Failed to sync")
-       }
-
-       return nil
-}
-
 type repoInfo struct {
        installedVer *newtutil.RepoVersion // nil if not installed.
        commitHash   string
@@ -953,6 +901,15 @@ func (inst *Installer) gatherInfo(r *repo.Repo,
                return ri
        }
 
+       if vm != nil {
+               // The caller requested a remote query.  Download this repo's 
latest
+               // `repository.yml` file.
+               if err := r.DownloadDesc(); err != nil {
+                       ri.errorText = strings.TrimSpace(err.Error())
+                       return ri
+               }
+       }
+
        commitHash, err := r.CurrentHash()
        if err != nil {
                ri.errorText = strings.TrimSpace(err.Error())
@@ -995,6 +952,13 @@ func (inst *Installer) Info(repos []*repo.Repo, remote 
bool) error {
        var vmp *deprepo.VersionMap
 
        if remote {
+               // Fetch the latest for all repos.
+               for _, r := range repos {
+                       if err := r.DownloadDesc(); err != nil {
+                               return err
+                       }
+               }
+
                vm, err := inst.calcVersionMap(repos)
                if err != nil {
                        return err
diff --git a/newt/project/project.go b/newt/project/project.go
index 4707934..e67adb2 100644
--- a/newt/project/project.go
+++ b/newt/project/project.go
@@ -282,26 +282,6 @@ func (proj *Project) InstallIf(
        }
 }
 
-// Syncs (i.e., applies `git pull` to) repos matching the specified predicate.
-func (proj *Project) SyncIf(
-       force bool, ask bool, 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 to sync.
-       repoList := proj.SelectRepos(predicate)
-
-       inst, err := install.NewInstaller(proj.repos, proj.rootRepoReqs)
-       if err != nil {
-               return err
-       }
-
-       return inst.Sync(repoList, force, ask)
-}
-
 func (proj *Project) InfoIf(predicate func(r *repo.Repo) bool,
        remote bool) error {
 
@@ -426,7 +406,9 @@ func (proj *Project) loadRepoDeps(download bool) error {
                                                                return nil, err
                                                        }
                                                }
-                                               proj.repos[dep.Name] = depRepo
+                                               if err := 
proj.addRepo(depRepo); err != nil {
+                                                       return nil, err
+                                               }
                                        }
                                        newRepos = append(newRepos, depRepo)
 
@@ -519,6 +501,17 @@ func (proj *Project) verifyNewtCompat() error {
        return nil
 }
 
+// addRepo Adds an entry to the project's repo map.  It clones the repo if it
+// does not exist locally.
+func (proj *Project) addRepo(r *repo.Repo) error {
+       if err := r.EnsureExists(); err != nil {
+               return err
+       }
+
+       proj.repos[r.Name()] = r
+       return nil
+}
+
 func (proj *Project) loadConfig() error {
        yc, err := config.ReadFile(proj.BasePath + "/" + PROJECT_FILE_NAME)
        if err != nil {
@@ -565,7 +558,9 @@ func (proj *Project) loadConfig() error {
                                        repoName, fields["vers"], err.Error())
                        }
 
-                       proj.repos[repoName] = r
+                       if err := proj.addRepo(r); err != nil {
+                               return err
+                       }
                        proj.rootRepoReqs[repoName] = verReqs
                }
        }
diff --git a/newt/repo/repo.go b/newt/repo/repo.go
index feae366..1c566b0 100644
--- a/newt/repo/repo.go
+++ b/newt/repo/repo.go
@@ -228,12 +228,16 @@ func (r *Repo) CheckExists() bool {
 
 func (r *Repo) updateRepo(commit string) error {
        // Clone the repo if it doesn't exist.
-       if err := r.ensureExists(); err != nil {
+       if err := r.EnsureExists(); err != nil {
                return err
        }
 
        // Fetch and checkout the specified commit.
-       if err := r.downloader.Pull(r.Path(), commit); err != nil {
+       if err := r.downloader.Fetch(r.Path()); err != nil {
+               return util.FmtNewtError(
+                       "Error updating \"%s\": %s", r.Name(), err.Error())
+       }
+       if err := r.downloader.Checkout(r.Path(), commit); err != nil {
                return util.FmtNewtError(
                        "Error updating \"%s\": %s", r.Name(), err.Error())
        }
@@ -276,71 +280,26 @@ func (r *Repo) Upgrade(ver newtutil.RepoVersion) error {
        return nil
 }
 
-// @return bool                 True if the sync succeeded.
-// @return error                Fatal error.
-func (r *Repo) Sync(ver newtutil.RepoVersion) (bool, error) {
-       // Sync is only allowed if a branch is checked out.
-       branch, err := r.downloader.CurrentBranch(r.localPath)
-       if err != nil {
-               return false, err
-       }
-
-       if branch == "" {
-               commits, err := r.CurrentCommits()
-               if err != nil {
-                       return false, err
-               }
-
-               util.StatusMessage(util.VERBOSITY_DEFAULT,
-                       "Skipping \"%s\": not using a branch 
(current-commits=%v)\n",
-                       r.Name(), commits)
-               return false, nil
-       }
-
-       // Determine the upstream associated with the current branch.  This is 
the
-       // upstream that will be pulled from.
-       upstream, err := r.downloader.UpstreamFor(r.localPath, branch)
-       if err != nil {
-               return false, err
-       }
-       if upstream == "" {
-               util.StatusMessage(util.VERBOSITY_QUIET,
-                       "Failed to sync repo \"%s\": no upstream being tracked 
"+
-                               "(branch=%s)\n",
-                       r.Name(), branch)
-               return false, nil
-       }
-
-       util.StatusMessage(util.VERBOSITY_DEFAULT,
-               "Syncing repository \"%s\" (%s)... ", r.Name(), upstream)
-
-       // Pull from upstream.
-       err = r.updateRepo(branch)
-       if err == nil {
-               util.StatusMessage(util.VERBOSITY_DEFAULT, "success\n")
-               return true, nil
-       } else {
-               util.StatusMessage(util.VERBOSITY_QUIET, "failed: %s\n",
-                       strings.TrimSpace(err.Error()))
-               return false, nil
-       }
-}
-
 // Fetches all remotes and downloads an up to date copy of `repository.yml`
 // from master.  The repo object is then populated with the contents of the
 // downladed file.  If this repo has already had its descriptor updated, this
 // function is a no-op.
 func (r *Repo) UpdateDesc() (bool, error) {
-       var err error
-
        if r.updated {
                return false, nil
        }
 
        util.StatusMessage(util.VERBOSITY_VERBOSE, "[%s]:\n", r.Name())
 
+       // Make sure the repo's "origin" remote points to the correct URL.  
This is
+       // necessary in case the user changed his `project.yml` file to point 
to a
+       // different fork.
+       if err := r.downloader.FixupOrigin(r.localPath); err != nil {
+               return false, err
+       }
+
        // Download `repository.yml`.
-       if err = r.DownloadDesc(); err != nil {
+       if err := r.DownloadDesc(); err != nil {
                return false, err
        }
 
@@ -354,7 +313,7 @@ func (r *Repo) UpdateDesc() (bool, error) {
        return true, nil
 }
 
-func (r *Repo) ensureExists() error {
+func (r *Repo) EnsureExists() error {
        // Clone the repo if it doesn't exist.
        if !r.CheckExists() {
                if err := r.downloadRepo("master"); err != nil {
@@ -362,13 +321,6 @@ func (r *Repo) ensureExists() error {
                }
        }
 
-       // Make sure the repo's "origin" remote points to the correct URL.  
This is
-       // necessary in case the user changed his `project.yml` file to point 
to a
-       // different fork.
-       if err := r.downloader.FixupOrigin(r.localPath); err != nil {
-               return err
-       }
-
        return nil
 }
 
@@ -376,7 +328,7 @@ func (r *Repo) downloadFile(commit string, srcPath string) 
(string, error) {
        dl := r.downloader
 
        // Clone the repo if it doesn't exist.
-       if err := r.ensureExists(); err != nil {
+       if err := r.EnsureExists(); err != nil {
                return "", err
        }
 
@@ -435,6 +387,16 @@ func (r *Repo) DownloadDesc() error {
        return nil
 }
 
+// IsDetached indicates whether a repo is in a "detached head" state.
+func (r *Repo) IsDetached() (bool, error) {
+       branch, err := r.downloader.CurrentBranch(r.Path())
+       if err != nil {
+               return false, err
+       }
+
+       return branch == "", nil
+}
+
 func parseRepoDepMap(depName string,
        repoMapYml interface{}) (map[string]*RepoDependency, error) {
 
diff --git a/newt/repo/version.go b/newt/repo/version.go
index 580a9cf..70f437f 100644
--- a/newt/repo/version.go
+++ b/newt/repo/version.go
@@ -78,18 +78,15 @@ func normalizeCommit(commit string) string {
 func (r *Repo) CurrentHash() (string, error) {
        dl := r.downloader
        if dl == nil {
-               return "",
-                       util.FmtNewtError("No downloader for %s",
-                               r.Name())
+               return "", util.FmtNewtError("No downloader for %s", r.Name())
        }
 
-       commit, err := dl.HashFor(r.Path(), "HEAD")
+       hash, err := dl.HashFor(r.Path(), "HEAD")
        if err != nil {
-               return "",
-                       util.FmtNewtError("Error finding current hash for 
\"%s\": %s",
-                               r.Name(), err.Error())
+               return "", err
        }
-       return commit, nil
+
+       return hash, nil
 }
 
 // Retrieves all commit strings corresponding to the repo's current state.
@@ -293,10 +290,6 @@ func (r *Repo) NormalizeVerReqs(verReqs 
[]newtutil.RepoVersionReq) (
 func (r *Repo) VersionsEqual(v1 newtutil.RepoVersion,
        v2 newtutil.RepoVersion) bool {
 
-       if newtutil.CompareRepoVersions(v1, v2) == 0 {
-               return true
-       }
-
        h1, err := r.HashFromVer(v1)
        if err != nil {
                return false
@@ -398,7 +391,7 @@ func (r *Repo) inferVersion(commit string, vyVer 
*newtutil.RepoVersion) (
                                "Version mismatch in %s:%s; repository.yml:%s, 
version.yml:%s",
                                r.Name(), commit, versString(ryVers), 
vyVer.String())
                } else {
-                       // If the set of commits don't match a version from
+                       // If the set of commits doesn't contain a version from
                        // `repository.yml`, record the commit hash in the 
version
                        // specifier.  This will distinguish the returned 
version from its
                        // corresponding official release.
@@ -436,6 +429,9 @@ func (r *Repo) InstalledVersion() (*newtutil.RepoVersion, 
error) {
        if err != nil {
                return nil, err
        }
+       if hash == "" {
+               return nil, nil
+       }
 
        ver, err := r.inferVersion(hash, vyVer)
        if err != nil {

Reply via email to