From b0a6391f1c1868c55a22fad0aeac53b0430bacb0 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 17 Jan 2012 18:10:16 +0000 Subject: [PATCH] Fix --force implementation to not prompt but still run hooks I found out that some devs thought that -f just skipped the prompt, but it did in fact also skip the presubmit checks. Make -f do what it should, no prompt and just return if the presubmit check failed. R=cmp@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/9240009 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@117918 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/git_cl.py b/git_cl.py index 9ba906269..c24fdb6af 100755 --- a/git_cl.py +++ b/git_cl.py @@ -550,7 +550,7 @@ or verify this branch is set up to track another (via the --track argument to self.SetWatchers(watchlist.GetWatchersForPaths(files)) try: - output = presubmit_support.DoPresubmitChecks(change, committing, + return presubmit_support.DoPresubmitChecks(change, committing, verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin, default_presubmit=None, may_prompt=may_prompt, rietveld_obj=self.RpcServer()) @@ -559,13 +559,6 @@ or verify this branch is set up to track another (via the --track argument to ('%s\nMaybe your depot_tools is out of date?\n' 'If all fails, contact maruel@') % e) - # TODO(dpranke): We should propagate the error out instead of calling - # exit(). - if not output.should_continue(): - sys.exit(1) - - return output - def CloseIssue(self): """Updates the description and closes the issue.""" issue = int(self.GetIssue()) @@ -909,15 +902,16 @@ def CMDupload(parser, args): base_branch = cl.GetUpstreamBranch() args = [base_branch + "..."] - if not options.bypass_hooks and not options.force: + if not options.bypass_hooks: hook_results = cl.RunHook(committing=False, upstream_branch=base_branch, - may_prompt=True, + may_prompt=not options.force, verbose=options.verbose, author=None) + if not hook_results.should_continue(): + return 1 if not options.reviewers and hook_results.reviewers: options.reviewers = hook_results.reviewers - # --no-ext-diff is broken in some versions of Git, so try to work around # this by overriding the environment (but there is still a problem if the # git config key "diff.external" is used). @@ -1068,24 +1062,29 @@ def SendUpstream(parser, args, cmd): 'before attempting to %s.' % (base_branch, cmd)) return 1 - if not options.bypass_hooks and not options.force: + if not options.bypass_hooks: author = None if options.contributor: author = re.search(r'\<(.*)\>', options.contributor).group(1) - cl.RunHook(committing=True, upstream_branch=base_branch, - may_prompt=True, verbose=options.verbose, - author=author) + hook_results = cl.RunHook( + committing=True, + upstream_branch=base_branch, + may_prompt=not options.force, + verbose=options.verbose, + author=author) + if not hook_results.should_continue(): + return 1 if cmd == 'dcommit': # Check the tree status if the tree status URL is set. status = GetTreeStatus() if 'closed' == status: - print ('The tree is closed. Please wait for it to reopen. Use ' - '"git cl dcommit -f" to commit on a closed tree.') + print('The tree is closed. Please wait for it to reopen. Use ' + '"git cl dcommit --bypass-hooks" to commit on a closed tree.') return 1 elif 'unknown' == status: - print ('Unable to determine tree status. Please verify manually and ' - 'use "git cl dcommit -f" to commit on a closed tree.') + print('Unable to determine tree status. Please verify manually and ' + 'use "git cl dcommit --bypass-hooks" to commit on a closed tree.') else: breakpad.SendStack( 'GitClHooksBypassedCommit',