upload: Allow user to configure unusual commit threshold
Add a per-remote option `uploadwarningthreshold` allowing the user to
override how many commits can be uploaded prior to a warning being
displayed.
Change-Id: Ia7e1b2c7de89a0bf9ca1c24cc83dc595b3667437
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/354375
Tested-by: David Greenaway <dgreenaway@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
diff --git a/docs/internal-fs-layout.md b/docs/internal-fs-layout.md
index a9bd1d2..8be6178 100644
--- a/docs/internal-fs-layout.md
+++ b/docs/internal-fs-layout.md
@@ -222,23 +222,24 @@
The `[branch]` settings are updated by `repo start` and `git branch`.
-| Setting | Subcommands | Use/Meaning |
-|-------------------------------|---------------|-------------|
-| review.\<url\>.autocopy | upload | Automatically add to `--cc=<value>` |
-| review.\<url\>.autoreviewer | upload | Automatically add to `--reviewers=<value>` |
-| review.\<url\>.autoupload | upload | Automatically answer "yes" or "no" to all prompts |
-| review.\<url\>.uploadhashtags | upload | Automatically add to `--hashtag=<value>` |
-| review.\<url\>.uploadlabels | upload | Automatically add to `--label=<value>` |
-| review.\<url\>.uploadnotify | upload | [Notify setting][upload-notify] to use |
-| review.\<url\>.uploadtopic | upload | Default [topic] to use |
-| review.\<url\>.username | upload | Override username with `ssh://` review URIs |
-| remote.\<remote\>.fetch | sync | Set of refs to fetch |
-| remote.\<remote\>.projectname | \<network\> | The name of the project as it exists in Gerrit review |
-| remote.\<remote\>.pushurl | upload | The base URI for pushing CLs |
-| remote.\<remote\>.review | upload | The URI of the Gerrit review server |
-| remote.\<remote\>.url | sync & upload | The URI of the git project to fetch |
-| branch.\<branch\>.merge | sync & upload | The branch to merge & upload & track |
-| branch.\<branch\>.remote | sync & upload | The remote to track |
+| Setting | Subcommands | Use/Meaning |
+|---------------------------------------|---------------|-------------|
+| review.\<url\>.autocopy | upload | Automatically add to `--cc=<value>` |
+| review.\<url\>.autoreviewer | upload | Automatically add to `--reviewers=<value>` |
+| review.\<url\>.autoupload | upload | Automatically answer "yes" or "no" to all prompts |
+| review.\<url\>.uploadhashtags | upload | Automatically add to `--hashtag=<value>` |
+| review.\<url\>.uploadlabels | upload | Automatically add to `--label=<value>` |
+| review.\<url\>.uploadnotify | upload | [Notify setting][upload-notify] to use |
+| review.\<url\>.uploadtopic | upload | Default [topic] to use |
+| review.\<url\>.uploadwarningthreshold | upload | Warn when attempting to upload more than this many CLs |
+| review.\<url\>.username | upload | Override username with `ssh://` review URIs |
+| remote.\<remote\>.fetch | sync | Set of refs to fetch |
+| remote.\<remote\>.projectname | \<network\> | The name of the project as it exists in Gerrit review |
+| remote.\<remote\>.pushurl | upload | The base URI for pushing CLs |
+| remote.\<remote\>.review | upload | The URI of the Gerrit review server |
+| remote.\<remote\>.url | sync & upload | The URI of the git project to fetch |
+| branch.\<branch\>.merge | sync & upload | The branch to merge & upload & track |
+| branch.\<branch\>.remote | sync & upload | The remote to track |
## ~/ dotconfig layout
diff --git a/subcmds/upload.py b/subcmds/upload.py
index 0ad3ce2..ceab646 100644
--- a/subcmds/upload.py
+++ b/subcmds/upload.py
@@ -17,6 +17,7 @@
import optparse
import re
import sys
+from typing import List
from command import DEFAULT_LOCAL_JOBS, InteractiveCommand
from editor import Editor
@@ -24,21 +25,54 @@
from git_command import GitCommand
from git_refs import R_HEADS
from hooks import RepoHook
+from project import ReviewableBranch
-UNUSUAL_COMMIT_THRESHOLD = 5
+_DEFAULT_UNUSUAL_COMMIT_THRESHOLD = 5
-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 = input("If you are sure you intend to do this, type 'yes': ").strip()
- return answer == "yes"
+def _VerifyPendingCommits(branches: List[ReviewableBranch]) -> bool:
+ """Perform basic safety checks on the given set of branches.
+
+ Ensures that each branch does not have a "large" number of commits
+ and, if so, prompts the user to confirm they want to proceed with
+ the upload.
+
+ Returns true if all branches pass the safety check or the user
+ confirmed. Returns false if the upload should be aborted.
+ """
+
+ # Determine if any branch has a suspicious number of commits.
+ many_commits = False
+ for branch in branches:
+ # Get the user's unusual threshold for the branch.
+ #
+ # Each branch may be configured to have a different threshold.
+ remote = branch.project.GetBranch(branch.name).remote
+ key = f'review.{remote.review}.uploadwarningthreshold'
+ threshold = branch.project.config.GetInt(key)
+ if threshold is None:
+ threshold = _DEFAULT_UNUSUAL_COMMIT_THRESHOLD
+
+ # If the branch has more commits than the threshold, show a warning.
+ if len(branch.commits) > threshold:
+ many_commits = True
+ break
+
+ # If any branch has many commits, prompt the user.
+ if many_commits:
+ if len(branches) > 1:
+ 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 = input(
+ "If you are sure you intend to do this, type 'yes': ").strip()
+ return answer == 'yes'
+
+ return True
def _die(fmt, *args):
@@ -149,6 +183,13 @@
Control e-mail notifications when uploading.
https://gerrit-review.googlesource.com/Documentation/user-upload.html#notify
+review.URL.uploadwarningthreshold:
+
+Repo will warn you if you are attempting to upload a large number
+of commits in one or more branches. By default, the threshold
+is five commits. This option allows you to override the warning
+threshold to a different value.
+
# References
Gerrit Code Review: https://www.gerritcodereview.com/
@@ -261,16 +302,15 @@
else:
answer = sys.stdin.readline().strip().lower()
answer = answer in ('y', 'yes', '1', 'true', 't')
+ if not answer:
+ _die("upload aborted by user")
- if not opt.yes and answer:
- if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD:
- answer = _ConfirmManyUploads()
-
- if answer:
- self._UploadAndReport(opt, [branch], people)
- else:
+ # Perform some basic safety checks prior to uploading.
+ if not opt.yes and not _VerifyPendingCommits([branch]):
_die("upload aborted by user")
+ self._UploadAndReport(opt, [branch], people)
+
def _MultipleBranches(self, opt, pending, people):
projects = {}
branches = {}
@@ -337,15 +377,9 @@
if not todo:
_die("nothing uncommented for upload")
- if not opt.yes:
- 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")
+ # Perform some basic safety checks prior to uploading.
+ if not opt.yes and not _VerifyPendingCommits(todo):
+ _die("upload aborted by user")
self._UploadAndReport(opt, todo, people)
