diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 792886163..2794702d0 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -34,6 +34,21 @@ _PRIMARY_EXPORT_TARGETS = [ '//:translator', ] + +def _SplitIntoMultipleCommits(description_text): + paragraph_split_pattern = r"(?m)(^\s*$\n)" + multiple_paragraphs = re.split(paragraph_split_pattern, description_text) + multiple_commits = [""] + change_id_pattern = re.compile(r"(?m)^Change-Id: [a-zA-Z0-9]*$") + for paragraph in multiple_paragraphs: + multiple_commits[-1] += paragraph + if change_id_pattern.search(paragraph): + multiple_commits.append("") + if multiple_commits[-1] == "": + multiple_commits.pop() + return multiple_commits + + def _CheckCommitMessageFormatting(input_api, output_api): def _IsLineBlank(line): @@ -50,19 +65,6 @@ def _CheckCommitMessageFormatting(input_api, output_api): def _IsTagLine(line): return ":" in line - def _SplitIntoMultipleCommits(description_text): - paragraph_split_pattern = r"((?m)^\s*$\n)" - multiple_paragraphs = re.split(paragraph_split_pattern, description_text) - multiple_commits = [""] - change_id_pattern = re.compile(r"(?m)^Change-Id: [a-zA-Z0-9]*$") - for paragraph in multiple_paragraphs: - multiple_commits[-1] += paragraph - if change_id_pattern.search(paragraph): - multiple_commits.append("") - if multiple_commits[-1] == "": - multiple_commits.pop() - return multiple_commits - def _CheckTabInCommit(lines): return all([line.find("\t") == -1 for line in lines]) @@ -329,7 +331,7 @@ def _CheckExportValidity(input_api, output_api): def _CheckTabsInSourceFiles(input_api, output_api): - """Forbids tab characters in source files due to a WebKit repo requirement. """ + """Forbids tab characters in source files due to a WebKit repo requirement.""" def implementation_and_headers_including_third_party(f): # Check third_party files too, because WebKit's checks don't make exceptions. @@ -364,7 +366,7 @@ def is_ascii(s): def _CheckNonAsciiInSourceFiles(input_api, output_api): - """Forbids non-ascii characters in source files. """ + """Forbids non-ascii characters in source files.""" def implementation_and_headers(f): return input_api.FilterSourceFile( @@ -389,7 +391,7 @@ def _CheckNonAsciiInSourceFiles(input_api, output_api): def _CheckCommentBeforeTestInTestFiles(input_api, output_api): - """Require a comment before TEST_P() and other tests. """ + """Require a comment before TEST_P() and other tests.""" def test_files(f): return input_api.FilterSourceFile( @@ -424,6 +426,42 @@ def _CheckCommentBeforeTestInTestFiles(input_api, output_api): return [] +def _CheckShaderVersionInShaderLangHeader(input_api, output_api): + """Requires an update to ANGLE_SH_VERSION when ShaderLang.h or ShaderVars.h change.""" + + def headers(f): + return input_api.FilterSourceFile( + f, + files_to_check=(r'^include\/GLSLANG\/ShaderLang.h$', + r'^include\/GLSLANG\/ShaderVars.h$')) + + headers_changed = input_api.AffectedSourceFiles(headers) + if len(headers_changed) == 0: + return [] + + # Skip this check for reverts and rolls. Unlike + # _CheckCommitMessageFormatting, relands are still checked because the + # original change might have incremented the version correctly, but the + # rebase over a new version could accidentally remove that (because another + # change in the meantime identically incremented it). + git_output = input_api.change.DescriptionText() + multiple_commits = _SplitIntoMultipleCommits(git_output) + for commit in multiple_commits: + if commit.startswith('Revert') or commit.startswith('Roll'): + return [] + + diffs = '\n'.join(f.GenerateScmDiff() for f in headers_changed) + versions = dict(re.findall(r'^([-+])#define ANGLE_SH_VERSION\s+(\d+)', diffs, re.M)) + + if len(versions) != 2 or int(versions['+']) <= int(versions['-']): + return [ + output_api.PresubmitError( + 'ANGLE_SH_VERSION should be incremented when ShaderLang.h or ShaderVars.h change.', + ) + ] + return [] + + def _CheckGClientExists(input_api, output_api, search_limit=None): presubmit_path = pathlib.Path(input_api.PresubmitLocalPath()) @@ -453,6 +491,7 @@ def CheckChangeOnUpload(input_api, output_api): results.extend(_CheckTabsInSourceFiles(input_api, output_api)) results.extend(_CheckNonAsciiInSourceFiles(input_api, output_api)) results.extend(_CheckCommentBeforeTestInTestFiles(input_api, output_api)) + results.extend(_CheckShaderVersionInShaderLangHeader(input_api, output_api)) results.extend(_CheckCodeGeneration(input_api, output_api)) results.extend(_CheckChangeHasBugField(input_api, output_api)) results.extend(input_api.canned_checks.CheckChangeHasDescription(input_api, output_api)) diff --git a/scripts/angle_presubmit_utils.py b/scripts/angle_presubmit_utils.py index 755155dd5..2da2bf666 100644 --- a/scripts/angle_presubmit_utils.py +++ b/scripts/angle_presubmit_utils.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright 2020 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -16,14 +16,27 @@ class Change_mock(): return self.description_text +class AffectedFile_mock(): + + def __init__(self, diff): + self.diff = diff + + def GenerateScmDiff(self): + return self.diff + + class InputAPI_mock(): - def __init__(self, description_text): + def __init__(self, description_text, source_files=[]): self.change = Change_mock(description_text) + self.source_files = source_files def PresubmitLocalPath(self): return self.cwd + def AffectedSourceFiles(self, source_filter): + return self.source_files + class _PresubmitResult(object): """Base class for result objects.""" diff --git a/scripts/angle_presubmit_utils_unittest.py b/scripts/angle_presubmit_utils_unittest.py old mode 100644 new mode 100755 index 822e46162..bdba75178 --- a/scripts/angle_presubmit_utils_unittest.py +++ b/scripts/angle_presubmit_utils_unittest.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright 2020 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -6,9 +6,10 @@ angle_presubmit_utils_unittest.py: Top-level unittest script for ANGLE presubmit checks. """ -import imp +import importlib.machinery import os import pathlib +import sys import tempfile import unittest from angle_presubmit_utils import * @@ -22,7 +23,9 @@ def SetCWDToAngleFolder(): SetCWDToAngleFolder() -PRESUBMIT = imp.load_source('PRESUBMIT', 'PRESUBMIT.py') +loader = importlib.machinery.SourceFileLoader('PRESUBMIT', 'PRESUBMIT.py') +PRESUBMIT = loader.load_module() + class CommitMessageFormattingCheckTest(unittest.TestCase): @@ -350,5 +353,96 @@ class GClientFileExistenceCheck(unittest.TestCase): self.assertEqual(len(errors), 0) +class CheckShaderVersionInShaderLangHeaderTest(unittest.TestCase): + + def __init__(self, *args, **kwargs): + super(CheckShaderVersionInShaderLangHeaderTest, self).__init__(*args, **kwargs) + self.output_api = OutputAPI_mock() + + def run_shader_version_check_presubmit(self, commit_msg, diffs): + affected_files = [AffectedFile_mock(diff) for diff in diffs] + input_api = InputAPI_mock(commit_msg, affected_files) + return PRESUBMIT._CheckShaderVersionInShaderLangHeader(input_api, self.output_api) + + def test_headers_not_changed(self): + errors = self.run_shader_version_check_presubmit('', []) + self.assertEqual(len(errors), 0) + + def test_shader_lang_changed_with_version_change(self): + shader_lang_diff = """-#define ANGLE_SH_VERSION 100 ++#define ANGLE_SH_VERSION 101 +""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff]) + self.assertEqual(len(errors), 0) + + def test_both_changed_with_version_change(self): + shader_lang_diff = """-#define ANGLE_SH_VERSION 100 ++#define ANGLE_SH_VERSION 101 +""" + shader_vars_diff = """-any change""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff, shader_vars_diff]) + self.assertEqual(len(errors), 0) + + def test_shader_lang_changed_with_no_version_change(self): + shader_lang_diff = """+some change""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff]) + self.assertEqual(len(errors), 1) + self.assertEqual( + errors[0], + self.output_api.PresubmitError( + 'ANGLE_SH_VERSION should be incremented when ShaderLang.h or ShaderVars.h change.') + ) + + def test_shader_lang_changed_with_no_version_change(self): + shader_lang_diff = """+some change + #define ANGLE_SH_VERSION 100 +-other changes""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff]) + self.assertEqual(len(errors), 1) + self.assertEqual( + errors[0], + self.output_api.PresubmitError( + 'ANGLE_SH_VERSION should be incremented when ShaderLang.h or ShaderVars.h change.') + ) + + def test_shader_lang_changed_with_version_cosmetic_change(self): + shader_lang_diff = """-#define ANGLE_SH_VERSION 100 ++#define ANGLE_SH_VERSION 100 // cosmetic change +""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff]) + self.assertEqual(len(errors), 1) + self.assertEqual( + errors[0], + self.output_api.PresubmitError( + 'ANGLE_SH_VERSION should be incremented when ShaderLang.h or ShaderVars.h change.') + ) + + def test_shader_lang_changed_with_version_decrement(self): + shader_lang_diff = """-#define ANGLE_SH_VERSION 100 ++#define ANGLE_SH_VERSION 99 +""" + + errors = self.run_shader_version_check_presubmit('', [shader_lang_diff]) + self.assertEqual(len(errors), 1) + self.assertEqual( + errors[0], + self.output_api.PresubmitError( + 'ANGLE_SH_VERSION should be incremented when ShaderLang.h or ShaderVars.h change.') + ) + + def test_shader_lang_changed_in_revert(self): + shader_lang_diff = """-#define ANGLE_SH_VERSION 100 ++#define ANGLE_SH_VERSION 99 +""" + + errors = self.run_shader_version_check_presubmit('Revert some change', [shader_lang_diff]) + self.assertEqual(len(errors), 0) + + if __name__ == '__main__': unittest.main()