git_cl sanity checks

git_cl.py

Index: git_cl.py
diff --git a/git_cl.py b/git_cl.py
index 21ade2a28e9987cfdc76425e64f8f0f027b6b1e6..bff528854ea74c59bfa9f4682efe78de52b30452 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -163,6 +163,35 @@ def is_dirty_git_tree(cmd):
     return True
   return False
 
+
+def git_cl_sanity_checks(upstream_git_obj):
+  """Checks git repo status and ensures diff is from local commits."""
+  
+  # Verify the commit we're diffing against is in our current branch. 
+  upstream_sha = RunGit(['rev-parse', '--verify', upstream_git_obj]).strip()
+  common_ancestor = RunGit(['merge-base', upstream_sha, 'HEAD']).strip()
+  if upstream_sha != common_ancestor:
+    print >> sys.stderr, (
+        'ERROR: %s is not in the current branch.  You may need to rebase your '
+        'tracking branch' % upstream_sha)
+    return False
+
+  # List the commits inside the diff, and verify they are all local.
+  commits_in_diff = RunGit(
+      ['rev-list', '^%s' % upstream_sha, 'HEAD']).splitlines()
+  commits_in_origin = RunGit(
+      ['rev-list', '^%s' % upstream_sha, 'remotes/origin/master']).splitlines()

[M-A Ruel, 2012/10/28 22:29:52]

I don't understand why remotes/origin/master is hardcoded here. I use git-cl
with git-svn clones frequently, where the "remote" is a svn-remote and I usually
diff against a remote named "trunk", not "origin/trunk".

+
+  common_commits = set(commits_in_diff) & set(commits_in_origin)
+  if common_commits:
+    print >> sys.stderr, (
+        'ERROR: Your diff contains %d commits already in origin/master.\n'
+        'Run "git log --oneline origin/master..HEAD" to get a list of commits '
+        'in the diff' % len(common_commits))
+    return False
+  return True
+
+
 def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards):
   """Return the corresponding git ref if |base_url| together with |glob_spec|
   matches the full |url|.
@@ -607,6 +636,9 @@ or verify this branch is set up to track another (via the --track argument to
     self.has_issue = False
 
   def GetChange(self, upstream_branch, author):
+    if not git_cl_sanity_checks(upstream_branch):
+      DieWithError('\nGit sanity check failure')
+
     root = RunCommand(['git', 'rev-parse', '--show-cdup']).strip() or '.'
     absroot = os.path.abspath(root)
 
@@ -1018,8 +1050,8 @@ def CMDpresubmit(parser, args):
   if args:
     base_branch = args[0]
   else:
-    # Default to diffing against the "upstream" branch.
-    base_branch = cl.GetUpstreamBranch()
+    # Default to diffing against the common ancestor of the upstream branch.
+    base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip()
 
   cl.RunHook(committing=not options.upload, upstream_branch=base_branch,
              may_prompt=False, verbose=options.verbose,
@@ -1216,9 +1248,9 @@ def CMDupload(parser, args):
     # TODO(ukai): is it ok for gerrit case?
     base_branch = args[0]
   else:
-    # Default to diffing against the "upstream" branch.
-    base_branch = cl.GetUpstreamBranch()
-    args = [base_branch + "..."]
+    # Default to diffing against common ancestor of upstream branch
+    base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip()
+    args = [base_branch]
 
   if not options.bypass_hooks:
     hook_results = cl.RunHook(committing=False, upstream_branch=base_branch,
@@ -1656,7 +1688,9 @@ def CMDtry(parser, args):
   # Process --bot and --testfilter.
   if not options.bot:
     # Get try slaves from PRESUBMIT.py files if not specified.
-    change = cl.GetChange(cl.GetUpstreamBranch(), None)
+    change = cl.GetChange(
+      RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip(),
+      None)
     options.bot = presubmit_support.DoGetTrySlaves(
         change,
         change.LocalPaths(),