Presubmit: Verify ANGLE_SH_VERSION update

... when ShaderLang.h or ShaderVars.h change.

Bug: angleproject:7279
Change-Id: I01f6035ad41a4d92cc9e5d1e509731bb2ff29047
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3823619
Reviewed-by: Roman Lavrov <romanl@google.com>
Commit-Queue: Roman Lavrov <romanl@google.com>
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>

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))

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."""

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()