Revert "git-cl: Fix format --dry-run not working with --full."

This reverts commit fc132e61db.

Reason for revert: Causes all presubmit checks to return dirty
format because of a non-zero diff output.

Original change's description:
> git-cl: Fix format --dry-run not working with --full.
> 
> The --full option would skip setting the return value of the format
> command when used with --dry-run. This in turn would cause the
> presubmit check to always pass when --full was enabled in a repo by
> default.
> 
> Was discovered in the ANGLE repository.
> 
> Bug: angleproject:4241
> Change-Id: Ie6cb423a6818c1e26781d77938a0dd22c02b4c16
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1987835
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Auto-Submit: Jamie Madill <jmadill@chromium.org>

TBR=ehmaldonado@chromium.org,jmadill@chromium.org

Change-Id: I0a4c51117ab20606f6dbb1f7a3ba40f87da1e939
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: angleproject:4241
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1993907
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
changes/07/1993907/2
Jamie Madill 6 years ago committed by Commit Bot
parent fc132e61db
commit 177710b35f

@ -5117,56 +5117,6 @@ def BuildGitDiffCmd(diff_type, upstream_commit, args, allow_prefix=False):
return diff_cmd
def _RunClangFormatDiff(opts, clang_diff_files, top_dir, upstream_commit):
"""Runs clang-format-diff and sets a return value if necessary."""
if not clang_diff_files:
return 0
# Set to 2 to signal to CheckPatchFormatted() that this patch isn't
# formatted. This is used to block during the presubmit.
return_value = 0
# Locate the clang-format binary in the checkout
try:
clang_format_tool = clang_format.FindClangFormatToolInChromiumTree()
except clang_format.NotFoundError as e:
DieWithError(e)
if opts.full or settings.GetFormatFullByDefault():
cmd = [clang_format_tool]
if not opts.dry_run and not opts.diff:
cmd.append('-i')
stdout = RunCommand(cmd + clang_diff_files, cwd=top_dir)
if opts.diff:
sys.stdout.write(stdout)
if opts.dry_run and len(stdout) > 0:
return_value = 2
else:
env = os.environ.copy()
env['PATH'] = str(os.path.dirname(clang_format_tool))
try:
script = clang_format.FindClangFormatScriptInChromiumTree(
'clang-format-diff.py')
except clang_format.NotFoundError as e:
DieWithError(e)
cmd = [sys.executable, script, '-p0']
if not opts.dry_run and not opts.diff:
cmd.append('-i')
diff_cmd = BuildGitDiffCmd('-U0', upstream_commit, clang_diff_files)
diff_output = RunGit(diff_cmd)
stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env)
if opts.diff:
sys.stdout.write(stdout)
if opts.dry_run and len(stdout) > 0:
return_value = 2
return return_value
def MatchingFileType(file_name, extensions):
"""Returns True if the file name ends with one of the given extensions."""
return bool([ext for ext in extensions if file_name.lower().endswith(ext)])
@ -5263,8 +5213,45 @@ def CMDformat(parser, args):
top_dir = os.path.normpath(
RunGit(["rev-parse", "--show-toplevel"]).rstrip('\n'))
return_value = _RunClangFormatDiff(opts, clang_diff_files, top_dir,
upstream_commit)
# Set to 2 to signal to CheckPatchFormatted() that this patch isn't
# formatted. This is used to block during the presubmit.
return_value = 0
if clang_diff_files:
# Locate the clang-format binary in the checkout
try:
clang_format_tool = clang_format.FindClangFormatToolInChromiumTree()
except clang_format.NotFoundError as e:
DieWithError(e)
if opts.full or settings.GetFormatFullByDefault():
cmd = [clang_format_tool]
if not opts.dry_run and not opts.diff:
cmd.append('-i')
stdout = RunCommand(cmd + clang_diff_files, cwd=top_dir)
if opts.diff:
sys.stdout.write(stdout)
else:
env = os.environ.copy()
env['PATH'] = str(os.path.dirname(clang_format_tool))
try:
script = clang_format.FindClangFormatScriptInChromiumTree(
'clang-format-diff.py')
except clang_format.NotFoundError as e:
DieWithError(e)
cmd = [sys.executable, script, '-p0']
if not opts.dry_run and not opts.diff:
cmd.append('-i')
diff_cmd = BuildGitDiffCmd('-U0', upstream_commit, clang_diff_files)
diff_output = RunGit(diff_cmd)
stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env)
if opts.diff:
sys.stdout.write(stdout)
if opts.dry_run and len(stdout) > 0:
return_value = 2
# Similar code to above, but using yapf on .py files rather than clang-format
# on C/C++ files
@ -5349,7 +5336,7 @@ def CMDformat(parser, args):
stdout = RunCommand(command, cwd=top_dir)
if opts.dry_run and stdout:
return_value = 2
except dart_format.NotFoundError:
except dart_format.NotFoundError as e:
print('Warning: Unable to check Dart code formatting. Dart SDK not '
'found in this checkout. Files in other languages are still '
'formatted.')

@ -32,7 +32,6 @@ import metrics
# We have to disable monitoring before importing git_cl.
metrics.DISABLE_METRICS_COLLECTION = True
import clang_format
import gerrit_util
import git_cl
import git_common
@ -3442,52 +3441,20 @@ class CMDFormatTestCase(TestCase):
def setUp(self):
super(CMDFormatTestCase, self).setUp()
mock.patch('git_cl.RunCommand').start()
mock.patch('clang_format.FindClangFormatToolInChromiumTree').start()
mock.patch('clang_format.FindClangFormatScriptInChromiumTree').start()
mock.patch('git_cl.settings').start()
self._top_dir = tempfile.mkdtemp()
self.addCleanup(mock.patch.stopall)
def tearDown(self):
shutil.rmtree(self._top_dir)
super(CMDFormatTestCase, self).tearDown()
def _make_temp_file(self, fname, contents):
with open(os.path.join(self._top_dir, fname), 'w') as tf:
tf.write('\n'.join(contents))
def _make_yapfignore(self, contents):
self._make_temp_file('.yapfignore', contents)
with open(os.path.join(self._top_dir, '.yapfignore'), 'w') as yapfignore:
yapfignore.write('\n'.join(contents))
def _check_yapf_filtering(self, files, expected):
self.assertEqual(expected, git_cl._FilterYapfIgnoredFiles(
files, git_cl._GetYapfIgnorePatterns(self._top_dir)))
def testClangFormatDiff(self):
git_cl.RunCommand.return_value = 'error'
git_cl.settings.GetFormatFullByDefault.return_value = False
return_value = git_cl._RunClangFormatDiff(
mock.Mock(full=True, dry_run=True, diff=False), ['.'], self._top_dir,
'HEAD')
self.assertEqual(2, return_value)
return_value = git_cl._RunClangFormatDiff(
mock.Mock(full=False, dry_run=True, diff=False), ['.'], self._top_dir,
'HEAD')
self.assertEqual(2, return_value)
def testClangFormatNoDiff(self):
git_cl.RunCommand.return_value = ''
git_cl.settings.GetFormatFullByDefault.return_value = False
return_value = git_cl._RunClangFormatDiff(
mock.Mock(full=True, dry_run=True, diff=False), ['.'], self._top_dir,
'HEAD')
self.assertEqual(0, return_value)
return_value = git_cl._RunClangFormatDiff(
mock.Mock(full=False, dry_run=True, diff=False), ['.'], self._top_dir,
'HEAD')
self.assertEqual(0, return_value)
def testYapfignoreExplicit(self):
self._make_yapfignore(['foo/bar.py', 'foo/bar/baz.py'])
files = [

Loading…
Cancel
Save