Revert "pre-upload: support options for default enabled flags"
This reverts commit 04e1fc535837213fbe7b96ad8d6e7dfe54112aa8.
Reason for revert: Causing BRANCH field checks and others failing repo uploads in other projects.
Original change's description:
> pre-upload: support options for default enabled flags
>
> We pull out hook options when the hook is explicitly enabled, but not
> when it's implicitly enabled by default. We haven't had any hooks as
> those take options before so no one noticed.
>
> BUG=b:162982485
> TEST=unittests pass
>
> Change-Id: Ib94acf043b2b4053ae67e704610a9b0417c8cbf5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/2363878
> Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
> Tested-by: Mike Frysinger <vapier@chromium.org>
> Commit-Queue: Mike Frysinger <vapier@chromium.org>
Bug: b:162982485
Change-Id: I943a71b7a3bc1e804d0b13b142c2048f534245fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/2365219
Reviewed-by: David Burger <dburger@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
Tested-by: David Burger <dburger@chromium.org>
Commit-Queue: David Burger <dburger@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index 3a759f4..bdacbec 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -2063,26 +2063,20 @@
valid_keys = set(_HOOK_FLAGS.keys())
hooks = _HOOK_FLAGS.copy()
- hook_overrides = set(
- config.options(SECTION) if config.has_section(SECTION) else [])
-
- unknown_keys = hook_overrides - valid_keys
- if unknown_keys:
- raise ValueError(f'{_CONFIG_FILE}: [{SECTION}]: unknown keys: '
- f'{unknown_keys}')
+ options = config.options(SECTION) if config.has_section(SECTION) else []
enable_flags = []
disable_flags = []
- for flag in valid_keys:
- if flag in hook_overrides:
- try:
- enabled = config.getboolean(SECTION, flag)
- except ValueError as e:
- raise ValueError('Error: parsing flag "%s" in "%s" failed: %s' %
- (flag, _CONFIG_FILE, e))
- else:
- enabled = True
+ for flag in options:
+ if flag not in valid_keys:
+ raise ValueError('Error: unknown key "%s" in hook section of "%s"' %
+ (flag, _CONFIG_FILE))
+ try:
+ enabled = config.getboolean(SECTION, flag)
+ except ValueError as e:
+ raise ValueError('Error: parsing flag "%s" in "%s" failed: %s' %
+ (flag, _CONFIG_FILE, e))
if enabled:
enable_flags.append(flag)
else:
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 78d6794..7613973 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -8,7 +8,6 @@
from __future__ import print_function
-import configparser
import datetime
import os
import sys
@@ -1863,47 +1862,5 @@
self.assertTrue(called)
-class OverrideHooksProcessing(PreUploadTestCase):
- """Verify _get_override_hooks processing."""
-
- @staticmethod
- def parse(data):
- """Helper to create a config & parse it."""
- cfg = configparser.ConfigParser()
- cfg.read_string(data)
- return pre_upload._get_override_hooks(cfg)
-
- def testHooks(self):
- """Verify we reject unknown hook names (e.g. typos)."""
- with self.assertRaises(ValueError) as e:
- self.parse("""
-[Hook Overrides]
-foobar: true
-""")
- self.assertIn('foobar', str(e.exception))
-
- def testExplicitDisable(self):
- """Verify hooks disabled are disabled."""
- _, disabled = self.parse("""
-[Hook Overrides]
-tab_check: false
-""")
- self.assertIn(pre_upload._check_no_tabs, disabled)
-
- def testOptions(self):
- """Verify hook options are loaded."""
- enabled, _ = self.parse("""
-[Hook Overrides Options]
-keyword_check: --kw
-""")
- for func in enabled:
- if func.__name__ == 'keyword_check':
- self.assertIn('options', func.keywords)
- self.assertEqual(func.keywords['options'], ['--kw'])
- break
- else:
- self.fail('could not find "keyword_check" enabled hook')
-
-
if __name__ == '__main__':
cros_test_lib.main(module=__name__)
