Gerrit git cl upload: abort early if change is submitted or abandoned.
BUG=689652
R=agable@chromium.org,martiniss@chromium.org
Change-Id: Ib9f5b08f5b31a1519a5f5916042885967a263d88
Reviewed-on: https://chromium-review.googlesource.com/443325
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
diff --git a/git_cl.py b/git_cl.py
index 1deebe3..c06fc87 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -1503,10 +1503,11 @@
base_branch = self.GetCommonAncestorWithUpstream()
git_diff_args = [base_branch, 'HEAD']
- # Make sure authenticated to codereview before running potentially expensive
- # hooks. It is a fast, best efforts check. Codereview still can reject the
- # authentication during the actual upload.
+ # Fast best-effort checks to abort before running potentially
+ # expensive hooks if uploading is likely to fail anyway. Passing these
+ # checks does not guarantee that uploading will not fail.
self._codereview_impl.EnsureAuthenticated(force=options.force)
+ self._codereview_impl.EnsureCanUploadPatchset()
# Apply watchlists on upload.
change = self.GetChange(base_branch, None)
@@ -1757,6 +1758,15 @@
"""
raise NotImplementedError()
+ def EnsureCanUploadPatchset(self):
+ """Best effort check that uploading isn't supposed to fail for predictable
+ reasons.
+
+ This method should raise informative exception if uploading shouldn't
+ proceed.
+ """
+ pass
+
def CMDUploadChange(self, options, args, change):
"""Uploads a change to codereview."""
raise NotImplementedError()
@@ -2310,6 +2320,26 @@
cookie_auth.get_netrc_path(),
cookie_auth.get_new_password_message(git_host)))
+ def EnsureCanUploadPatchset(self):
+ """Best effort check that uploading isn't supposed to fail for predictable
+ reasons.
+
+ This method should raise informative exception if uploading shouldn't
+ proceed.
+ """
+ if not self.GetIssue():
+ return
+
+ # Warm change details cache now to avoid RPCs later, reducing latency for
+ # developers.
+ self.FetchDescription()
+
+ status = self._GetChangeDetail()['status']
+ if status in ('MERGED', 'ABANDONED'):
+ DieWithError('Change %s has been %s, new uploads are not allowed' %
+ (self.GetIssueURL(),
+ 'submitted' if status == 'MERGED' else 'abandoned'))
+
def _PostUnsetIssueProperties(self):
"""Which branch-specific properties to erase when unsetting issue."""
return ['gerritsquashhash']