From 879a9a5cf0f4ed61df6544949068babbee4f60e2 Mon Sep 17 00:00:00 2001 From: Dan Morrill Date: Tue, 4 May 2010 16:56:07 -0700 Subject: upload: Confirm unusually large number of uploaded commit Add a sentinel check to require a second explicit confirmation if the user is attempting to upload (or upload --replace) an unusually large number of commits. This may help the user to catch an accidentally incorrect rebase they had done previously. Change-Id: I12c4d102f90a631d6ad193486a70ffd520ef6ae0 --- subcmds/upload.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'subcmds/upload.py') diff --git a/subcmds/upload.py b/subcmds/upload.py index aea399b6..4dc11d28 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -20,6 +20,17 @@ from command import InteractiveCommand from editor import Editor from error import UploadError +UNUSUAL_COMMIT_THRESHOLD = 3 + +def _ConfirmManyUploads(multiple_branches=False): + if multiple_branches: + print "ATTENTION: One or more branches has an unusually high number of commits." + else: + print "ATTENTION: You are uploading an unusually high number of commits." + print "YOU PROBABLY DO NOT MEAN TO DO THIS. (Did you rebase across branches?)" + answer = raw_input("If you are sure you intend to do this, type 'yes': ").strip() + return answer == "yes" + def _die(fmt, *args): msg = fmt % args print >>sys.stderr, 'error: %s' % msg @@ -128,6 +139,10 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ answer = sys.stdin.readline().strip() answer = answer in ('y', 'Y', 'yes', '1', 'true', 't') + if answer: + if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD: + answer = _ConfirmManyUploads() + if answer: self._UploadAndReport([branch], people) else: @@ -192,6 +207,16 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ todo.append(branch) if not todo: _die("nothing uncommented for upload") + + many_commits = False + for branch in todo: + if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD: + many_commits = True + break + if many_commits: + if not _ConfirmManyUploads(multiple_branches=True): + _die("upload aborted by user") + self._UploadAndReport(todo, people) def _FindGerritChange(self, branch): @@ -258,6 +283,10 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ print >>sys.stderr, " use 'repo upload' without --replace" sys.exit(1) + if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD: + if not _ConfirmManyUploads(multiple_branches=True): + _die("upload aborted by user") + branch.replace_changes = to_replace self._UploadAndReport([branch], people) -- cgit v1.2.3-54-g00ecf From f0a9a1a30e60e92cec9bff4cae030478c276da4d Mon Sep 17 00:00:00 2001 From: Dan Morrill Date: Wed, 5 May 2010 08:18:35 -0700 Subject: upload: Move confirmation threshold from 3 to 5 commits Change-Id: I7275d195cf04f02694206b9f838540b0228ff5e1 --- subcmds/upload.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'subcmds/upload.py') diff --git a/subcmds/upload.py b/subcmds/upload.py index 4dc11d28..4fa5b432 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -20,7 +20,7 @@ from command import InteractiveCommand from editor import Editor from error import UploadError -UNUSUAL_COMMIT_THRESHOLD = 3 +UNUSUAL_COMMIT_THRESHOLD = 5 def _ConfirmManyUploads(multiple_branches=False): if multiple_branches: -- cgit v1.2.3-54-g00ecf From 08a3f68d38eec81dfa66f9ea05080c37c863f322 Mon Sep 17 00:00:00 2001 From: Ben Komalo Date: Thu, 15 Jul 2010 16:03:02 -0700 Subject: upload: Automatically --cc folks in review.URL.autocopy The upload command will read review.URL.autocopy from the project's configuration and append the list of e-mails specified to the --cc argument of the upload command if a non-empty --re argument was provided. Change-Id: I2424517d17dd3444b20f0e6a003be6e70b8904f6 Signed-off-by: Shawn O. Pearce --- subcmds/upload.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) (limited to 'subcmds/upload.py') diff --git a/subcmds/upload.py b/subcmds/upload.py index 4fa5b432..ba532461 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import re import sys @@ -83,6 +84,14 @@ to "true" then repo will assume you always answer "y" at the prompt, and will not prompt you further. If it is set to "false" then repo will assume you always answer "n", and will abort. +review.URL.autocopy: + +To automatically copy a user or mailing list to all uploaded reviews, +you can set a per-project or global Git option to do so. Specifically, +review.URL.autocopy can be set to a comma separated list of reviewers +who you always want copied on all uploads with a non-empty --re +argument. + The URL must match the review URL listed in the manifest XML file, or in the .git/config within the project. For example: @@ -92,6 +101,7 @@ or in the .git/config within the project. For example: [review "http://review.example.com/"] autoupload = true + autocopy = johndoe@company.com,my-team-alias@company.com References ---------- @@ -219,6 +229,19 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ self._UploadAndReport(todo, people) + def _AppendAutoCcList(self, branch, people): + """ + Appends the list of users in the CC list in the git project's config if a + non-empty reviewer list was found. + """ + + name = branch.name + project = branch.project + key = 'review.%s.autocopy' % project.GetBranch(name).remote.review + raw_list = project.config.GetString(key) + if not raw_list is None and len(people[0]) > 0: + people[1].extend([entry.strip() for entry in raw_list.split(',')]) + def _FindGerritChange(self, branch): last_pub = branch.project.WasPublished(branch.name) if last_pub is None: @@ -290,10 +313,13 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ branch.replace_changes = to_replace self._UploadAndReport([branch], people) - def _UploadAndReport(self, todo, people): + def _UploadAndReport(self, todo, original_people): have_errors = False for branch in todo: try: + people = copy.deepcopy(original_people) + self._AppendAutoCcList(branch, people) + branch.UploadForReview(people) branch.uploaded = True except UploadError, e: -- cgit v1.2.3-54-g00ecf From cc50bac8c7706082596d70756249d4964a67f281 Mon Sep 17 00:00:00 2001 From: Anthony Newnam Date: Thu, 8 Apr 2010 10:28:59 -0500 Subject: Warn users before uploading if there are local changes Change-Id: I231d7b6a3211e9f5ec71a542a0109b0c195d5e40 Signed-off-by: Shawn O. Pearce --- project.py | 21 +++++++++++++++++++++ subcmds/upload.py | 15 +++++++++++++++ 2 files changed, 36 insertions(+) (limited to 'subcmds/upload.py') diff --git a/project.py b/project.py index 956f45bf..4e8fa0e0 100644 --- a/project.py +++ b/project.py @@ -368,6 +368,27 @@ class Project(object): ## Status Display ## + def HasChanges(self): + """Returns true if there are uncommitted changes. + """ + self.work_git.update_index('-q', + '--unmerged', + '--ignore-missing', + '--refresh') + if self.IsRebaseInProgress(): + return True + + if self.work_git.DiffZ('diff-index', '--cached', HEAD): + return True + + if self.work_git.DiffZ('diff-files'): + return True + + if self.work_git.LsOthers(): + return True + + return False + def PrintWorkTreeStatus(self): """Prints the status of the repository to stdout. """ diff --git a/subcmds/upload.py b/subcmds/upload.py index ba532461..5a426113 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -320,6 +320,21 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ people = copy.deepcopy(original_people) self._AppendAutoCcList(branch, people) + # Check if there are local changes that may have been forgotten + if branch.project.HasChanges(): + key = 'review.%s.autoupload' % branch.project.remote.review + answer = branch.project.config.GetBoolean(key) + + # if they want to auto upload, let's not ask because it could be automated + if answer is None: + sys.stdout.write('Uncommitted changes in ' + branch.project.name + ' (did you forget to amend?). Continue uploading? (y/n) ') + a = sys.stdin.readline().strip().lower() + if a not in ('y', 'yes', 't', 'true', 'on'): + print >>sys.stderr, "skipping upload" + branch.uploaded = False + branch.error = 'User aborted' + continue + branch.UploadForReview(people) branch.uploaded = True except UploadError, e: -- cgit v1.2.3-54-g00ecf From a5ece0e0505324218f38af02a1fe046ca2bcc278 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 15 Jul 2010 16:52:42 -0700 Subject: upload -t: Automatically include local branch name If the -t flag is given to upload, the local branch name is automatically sent to Gerrit Code Review as the topic branch name for the change(s). This requires the server to be Gerrit Code Review v2.1.3-53-gd50c94e or later, which isn't widely deployed right now, so the default is opt-out. Change-Id: I034fcacb405b7cb909147152db427fe69dd7bcbf Signed-off-by: Shawn O. Pearce --- project.py | 17 +++++++++++++---- subcmds/upload.py | 21 ++++++++++++--------- 2 files changed, 25 insertions(+), 13 deletions(-) (limited to 'subcmds/upload.py') diff --git a/project.py b/project.py index 4e8fa0e0..1b5d9a67 100644 --- a/project.py +++ b/project.py @@ -149,10 +149,11 @@ class ReviewableBranch(object): R_HEADS + self.name, '--') - def UploadForReview(self, people): + def UploadForReview(self, people, auto_topic=False): self.project.UploadForReview(self.name, self.replace_changes, - people) + people, + auto_topic=auto_topic) def GetPublishedRefs(self): refs = {} @@ -555,7 +556,10 @@ class Project(object): return rb return None - def UploadForReview(self, branch=None, replace_changes=None, people=([],[])): + def UploadForReview(self, branch=None, + replace_changes=None, + people=([],[]), + auto_topic=False): """Uploads the named branch for code review. """ if branch is None: @@ -587,10 +591,15 @@ class Project(object): for e in people[1]: rp.append('--cc=%s' % sq(e)) + ref_spec = '%s:refs/for/%s' % (R_HEADS + branch.name, dest_branch) + if auto_topic: + ref_spec = ref_spec + '/' + branch.name + cmd = ['push'] cmd.append('--receive-pack=%s' % " ".join(rp)) cmd.append(branch.remote.SshReviewUrl(self.UserEmail)) - cmd.append('%s:refs/for/%s' % (R_HEADS + branch.name, dest_branch)) + cmd.append(ref_spec) + if replace_changes: for change_id,commit_id in replace_changes.iteritems(): cmd.append('%s:refs/changes/%s/new' % (commit_id, change_id)) diff --git a/subcmds/upload.py b/subcmds/upload.py index 5a426113..569e31c1 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -111,6 +111,9 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ """ def _Options(self, p): + p.add_option('-t', + dest='auto_topic', action='store_true', + help='Send local branch name to Gerrit Code Review') p.add_option('--replace', dest='replace', action='store_true', help='Upload replacement patchesets from this branch') @@ -121,7 +124,7 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ type='string', action='append', dest='cc', help='Also send email to these email addresses.') - def _SingleBranch(self, branch, people): + def _SingleBranch(self, opt, branch, people): project = branch.project name = branch.name remote = project.GetBranch(name).remote @@ -154,11 +157,11 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ answer = _ConfirmManyUploads() if answer: - self._UploadAndReport([branch], people) + self._UploadAndReport(opt, [branch], people) else: _die("upload aborted by user") - def _MultipleBranches(self, pending, people): + def _MultipleBranches(self, opt, pending, people): projects = {} branches = {} @@ -227,7 +230,7 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ if not _ConfirmManyUploads(multiple_branches=True): _die("upload aborted by user") - self._UploadAndReport(todo, people) + self._UploadAndReport(opt, todo, people) def _AppendAutoCcList(self, branch, people): """ @@ -311,9 +314,9 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ _die("upload aborted by user") branch.replace_changes = to_replace - self._UploadAndReport([branch], people) + self._UploadAndReport(opt, [branch], people) - def _UploadAndReport(self, todo, original_people): + def _UploadAndReport(self, opt, todo, original_people): have_errors = False for branch in todo: try: @@ -335,7 +338,7 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ branch.error = 'User aborted' continue - branch.UploadForReview(people) + branch.UploadForReview(people, auto_topic=opt.auto_topic) branch.uploaded = True except UploadError, e: branch.error = e @@ -391,6 +394,6 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ if not pending: print >>sys.stdout, "no branches ready for upload" elif len(pending) == 1 and len(pending[0][1]) == 1: - self._SingleBranch(pending[0][1][0], people) + self._SingleBranch(opt, pending[0][1][0], people) else: - self._MultipleBranches(pending, people) + self._MultipleBranches(opt, pending, people) -- cgit v1.2.3-54-g00ecf From 3575b8f8bdc5f15de23db82499e0ce152f634a19 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 15 Jul 2010 17:00:14 -0700 Subject: upload: Allow review.HOST.username to override email Some users might need to use a different login name than the local part of their email address for their Gerrit Code Review user account. Allow it to be overridden with the review.HOST.username configuration variable. Change-Id: I714469142ac7feadf09fee9c26680c0e09076b75 Signed-off-by: Shawn O. Pearce --- git_config.py | 5 ++++- subcmds/upload.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'subcmds/upload.py') diff --git a/git_config.py b/git_config.py index dc209ba8..1382d5db 100644 --- a/git_config.py +++ b/git_config.py @@ -531,8 +531,11 @@ class Remote(object): def SshReviewUrl(self, userEmail): if self.ReviewProtocol != 'ssh': return None + username = self._config.GetString('review.%s.username' % self.review) + if username is None: + username = userEmail.split("@")[0] return 'ssh://%s@%s:%s/%s' % ( - userEmail.split("@")[0], + username, self._review_host, self._review_port, self.projectname) diff --git a/subcmds/upload.py b/subcmds/upload.py index 569e31c1..b47b37b8 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -92,6 +92,11 @@ review.URL.autocopy can be set to a comma separated list of reviewers who you always want copied on all uploads with a non-empty --re argument. +review.URL.username: + +Override the username used to connect to Gerrit Code Review. +By default the local part of the email address is used. + The URL must match the review URL listed in the manifest XML file, or in the .git/config within the project. For example: -- cgit v1.2.3-54-g00ecf From 60829ba72fe81b1de1c1e9c6e0de486e9e90bddd Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Jul 2010 07:42:45 -0700 Subject: upload: Fix --replace flag --replace started to fail due to a Python error, I forgot to pass through the opt structure to the replace function. Change-Id: Ifcd7a0c715c3fd9070a4c58208612a626382de35 Signed-off-by: Shawn O. Pearce --- subcmds/upload.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'subcmds/upload.py') diff --git a/subcmds/upload.py b/subcmds/upload.py index b47b37b8..153b3ebe 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -262,7 +262,7 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ except: return "" - def _ReplaceBranch(self, project, people): + def _ReplaceBranch(self, opt, project, people): branch = project.CurrentBranch if not branch: print >>sys.stdout, "no branches ready for upload" @@ -388,7 +388,7 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ print >>sys.stderr, \ 'error: --replace requires exactly one project' sys.exit(1) - self._ReplaceBranch(project_list[0], people) + self._ReplaceBranch(opt, project_list[0], people) return for project in project_list: -- cgit v1.2.3-54-g00ecf From a0de6e8eab97f5dcdb2f51d4e09dd1568623ec58 Mon Sep 17 00:00:00 2001 From: Ficus Kirkpatrick Date: Fri, 22 Oct 2010 13:06:47 -0700 Subject: upload: Remove --replace option It hasn't been necessary for a long time, and its functionality can be accomplished with 'git push'. Change-Id: Ic00d3adbe4cee7be3955117489c69d6e90106559 --- project.py | 6 ----- subcmds/upload.py | 78 +------------------------------------------------------ 2 files changed, 1 insertion(+), 83 deletions(-) (limited to 'subcmds/upload.py') diff --git a/project.py b/project.py index ce85b863..01dc8678 100644 --- a/project.py +++ b/project.py @@ -111,7 +111,6 @@ class ReviewableBranch(object): self.project = project self.branch = branch self.base = base - self.replace_changes = None @property def name(self): @@ -151,7 +150,6 @@ class ReviewableBranch(object): def UploadForReview(self, people, auto_topic=False): self.project.UploadForReview(self.name, - self.replace_changes, people, auto_topic=auto_topic) @@ -557,7 +555,6 @@ class Project(object): return None def UploadForReview(self, branch=None, - replace_changes=None, people=([],[]), auto_topic=False): """Uploads the named branch for code review. @@ -600,9 +597,6 @@ class Project(object): cmd.append(branch.remote.SshReviewUrl(self.UserEmail)) cmd.append(ref_spec) - if replace_changes: - for change_id,commit_id in replace_changes.iteritems(): - cmd.append('%s:refs/changes/%s/new' % (commit_id, change_id)) if GitCommand(self, cmd, bare = True).Wait() != 0: raise UploadError('Upload failed') diff --git a/subcmds/upload.py b/subcmds/upload.py index 153b3ebe..1964bffa 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -47,7 +47,7 @@ class Upload(InteractiveCommand): common = True helpSummary = "Upload changes for code review" helpUsage=""" -%prog [--re --cc] {[]... | --replace } +%prog [--re --cc] []... """ helpDescription = """ The '%prog' command is used to send changes to the Gerrit Code @@ -67,12 +67,6 @@ added to the respective list of users, and emails are sent to any new users. Users passed as --reviewers must already be registered with the code review system, or the upload will fail. -If the --replace option is passed the user can designate which -existing change(s) in Gerrit match up to the commits in the branch -being uploaded. For each matched pair of change,commit the commit -will be added as a new patch set, completely replacing the set of -files and description associated with the change in Gerrit. - Configuration ------------- @@ -119,9 +113,6 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ p.add_option('-t', dest='auto_topic', action='store_true', help='Send local branch name to Gerrit Code Review') - p.add_option('--replace', - dest='replace', action='store_true', - help='Upload replacement patchesets from this branch') p.add_option('--re', '--reviewers', type='string', action='append', dest='reviewers', help='Request reviews from these people.') @@ -262,65 +253,6 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ except: return "" - def _ReplaceBranch(self, opt, project, people): - branch = project.CurrentBranch - if not branch: - print >>sys.stdout, "no branches ready for upload" - return - branch = project.GetUploadableBranch(branch) - if not branch: - print >>sys.stdout, "no branches ready for upload" - return - - script = [] - script.append('# Replacing from branch %s' % branch.name) - - if len(branch.commits) == 1: - change = self._FindGerritChange(branch) - script.append('[%-6s] %s' % (change, branch.commits[0])) - else: - for commit in branch.commits: - script.append('[ ] %s' % commit) - - script.append('') - script.append('# Insert change numbers in the brackets to add a new patch set.') - script.append('# To create a new change record, leave the brackets empty.') - - script = Editor.EditString("\n".join(script)).split("\n") - - change_re = re.compile(r'^\[\s*(\d{1,})\s*\]\s*([0-9a-f]{1,}) .*$') - to_replace = dict() - full_hashes = branch.unabbrev_commits - - for line in script: - m = change_re.match(line) - if m: - c = m.group(1) - f = m.group(2) - try: - f = full_hashes[f] - except KeyError: - print 'fh = %s' % full_hashes - print >>sys.stderr, "error: commit %s not found" % f - sys.exit(1) - if c in to_replace: - print >>sys.stderr,\ - "error: change %s cannot accept multiple commits" % c - sys.exit(1) - to_replace[c] = f - - if not to_replace: - print >>sys.stderr, "error: no replacements specified" - print >>sys.stderr, " use 'repo upload' without --replace" - sys.exit(1) - - if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD: - if not _ConfirmManyUploads(multiple_branches=True): - _die("upload aborted by user") - - branch.replace_changes = to_replace - self._UploadAndReport(opt, [branch], people) - def _UploadAndReport(self, opt, todo, original_people): have_errors = False for branch in todo: @@ -383,14 +315,6 @@ Gerrit Code Review: http://code.google.com/p/gerrit/ cc = _SplitEmails(opt.cc) people = (reviewers,cc) - if opt.replace: - if len(project_list) != 1: - print >>sys.stderr, \ - 'error: --replace requires exactly one project' - sys.exit(1) - self._ReplaceBranch(opt, project_list[0], people) - return - for project in project_list: avail = project.GetUploadableBranches() if avail: -- cgit v1.2.3-54-g00ecf