Merge "jiri: Return proper errors from hooks and git-hooks"
diff --git a/gitutil/.api b/gitutil/.api
index c86d89c..dcd0a23 100644
--- a/gitutil/.api
+++ b/gitutil/.api
@@ -44,6 +44,7 @@
pkg gitutil, method (*Git) Push(string, string, ...PushOpt) error
pkg gitutil, method (*Git) Rebase(string) error
pkg gitutil, method (*Git) RebaseAbort() error
+pkg gitutil, method (*Git) RefExistsOnBranch(string, string) bool
pkg gitutil, method (*Git) RemoteUrl(string) (string, error)
pkg gitutil, method (*Git) Remove(...string) error
pkg gitutil, method (*Git) RemoveUntrackedFiles() error
diff --git a/gitutil/git.go b/gitutil/git.go
index ee77840..c6a6675 100644
--- a/gitutil/git.go
+++ b/gitutil/git.go
@@ -536,6 +536,15 @@
return g.run("rebase", "--abort")
}
+// RefExistsOnBranch returns true if the given ref exists on the given branch.
+func (g *Git) RefExistsOnBranch(ref, branch string) bool {
+ args := []string{"merge-base", "--is-ancestor", ref, branch}
+ if err := g.run(args...); err != nil {
+ return false
+ }
+ return true
+}
+
// Remove removes the given files.
func (g *Git) Remove(fileNames ...string) error {
args := []string{"rm"}
diff --git a/project/project.go b/project/project.go
index d8a8cb8..dc47bdd 100644
--- a/project/project.go
+++ b/project/project.go
@@ -1450,13 +1450,20 @@
}
switch project.Protocol {
case "git":
- // Having a specific revision trumps everything else.
+ git := gitutil.New(jirix.NewSeq())
+ remoteBranch := "origin/" + project.RemoteBranch
+
+ // If a specific revision is set, make sure it exists on the specified
+ // remote branch, and reset to that revision.
if project.Revision != "HEAD" {
- return gitutil.New(jirix.NewSeq()).Reset(project.Revision)
+ if !git.RefExistsOnBranch(project.Revision, remoteBranch) {
+ return fmt.Errorf("revision %q does not exist on remote branch %q", project.Revision, remoteBranch)
+ }
+ return git.Reset(project.Revision)
}
- // If no revision, reset to the configured remote branch, or master
- // if no remote branch.
- return gitutil.New(jirix.NewSeq()).Reset("origin/" + project.RemoteBranch)
+
+ // If no revision is set, reset to the remote branch.
+ return git.Reset(remoteBranch)
default:
return UnsupportedProtocolErr(project.Protocol)
}
@@ -2042,10 +2049,6 @@
if err := s.Chdir(tmpDir).Done(); err != nil {
return err
}
- // TODO(toddw): Why call Reset here, when resetProject is called just below?
- if err := gitutil.New(jirix.NewSeq()).Reset(op.project.Revision); err != nil {
- return err
- }
default:
return UnsupportedProtocolErr(op.project.Protocol)
}
@@ -2170,7 +2173,7 @@
}
func (op moveOperation) String() string {
- return fmt.Sprintf("move project %q located in %q to %q and advance it to %q", op.project.Name, op.source, op.destination, fmtRevision(op.project.Revision))
+ return fmt.Sprintf("move project %q located in %q to %q and advance it to %q of %q", op.project.Name, op.source, op.destination, fmtRevision(op.project.Revision), op.project.RemoteBranch)
}
func (op moveOperation) Test(jirix *jiri.X, updates *fsUpdates) error {
@@ -2214,7 +2217,7 @@
}
func (op updateOperation) String() string {
- return fmt.Sprintf("advance project %q located in %q to %q", op.project.Name, op.source, fmtRevision(op.project.Revision))
+ return fmt.Sprintf("advance project %q located in %q to %q of %q", op.project.Name, op.source, fmtRevision(op.project.Revision), op.project.RemoteBranch)
}
func (op updateOperation) Test(jirix *jiri.X, _ *fsUpdates) error {
diff --git a/project/project_test.go b/project/project_test.go
index a060e36..b4c69af 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -172,7 +172,7 @@
return revision
}
-// Fix the revision in the manifest file.
+// Set the revision in the manifest file.
func setRevisionForProject(t *testing.T, jirix *jiri.X, manifestDir, name, revision string) {
m, err := project.ManifestFromFile(jirix, filepath.Join(manifestDir, "v2", "default"))
if err != nil {
@@ -188,7 +188,28 @@
}
}
if !updated {
- t.Fatalf("failed to fix revision for project %v", name)
+ t.Fatalf("failed to set revision for project %v", name)
+ }
+ commitManifest(t, jirix, m, manifestDir)
+}
+
+// Set the remote branch in the manifest file.
+func setRemoteBranchForProject(t *testing.T, jirix *jiri.X, manifestDir, name, remoteBranch string) {
+ m, err := project.ManifestFromFile(jirix, filepath.Join(manifestDir, "v2", "default"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ updated := false
+ for i, p := range m.Projects {
+ if p.Name == name {
+ p.RemoteBranch = remoteBranch
+ m.Projects[i] = p
+ updated = true
+ break
+ }
+ }
+ if !updated {
+ t.Fatalf("failed to fix remote branch for project %v", name)
}
commitManifest(t, jirix, m, manifestDir)
}
@@ -523,14 +544,26 @@
t.Fatalf("%v", err)
}
- // Commit to a non-master branch of a remote project and check that
- // UpdateUniverse() can update the local project to point to a revision on
- // that branch.
+ // Commit to master and non-master branches of a remote project.
+ nonMasterBranch := "non_master"
writeReadme(t, jirix, remoteProjects[5], "master commit")
- createAndCheckoutBranch(t, jirix, remoteProjects[5], "non_master")
+ createAndCheckoutBranch(t, jirix, remoteProjects[5], nonMasterBranch)
writeReadme(t, jirix, remoteProjects[5], "non master commit")
remoteBranchRevision := currentRevision(t, jirix, remoteProjects[5])
+
+ // Set the revision to the non-master revision, but keep the remote branch
+ // set to master.
setRevisionForProject(t, jirix, remoteManifest, remoteProjects[5], remoteBranchRevision)
+ // Check that UpdateUniverse() fails when updating to a revision that does
+ // not occur on the remote branch (master).
+ if err := project.UpdateUniverse(jirix, true); err == nil {
+ t.Fatalf("expected project.UpdateUniverse() with revision that does not occur on remote branch to fail, but it did not")
+ }
+
+ // Set the project remote branch to the non-master branch with the revision.
+ setRemoteBranchForProject(t, jirix, remoteManifest, remoteProjects[5], nonMasterBranch)
+ // Check that UpdateUniverse() can update the local project to point to a
+ // revision on the non-master remote branch.
if err := project.UpdateUniverse(jirix, true); err != nil {
t.Fatalf("%v", err)
}
