diff --git a/.github/workflows/check-entangled-specs.yml b/.github/workflows/check-entangled-specs.yml index 486483ddecf..74beaf4a13f 100644 --- a/.github/workflows/check-entangled-specs.yml +++ b/.github/workflows/check-entangled-specs.yml @@ -31,5 +31,9 @@ jobs: - name: Get Python dependencies run: python3 -m pip install -r toolkit/scripts/requirements.txt - - name: Run entanglement checking script + # Run unit test for check_entangled_specs.py before invoking it + - name: Unit test for spec entanglement check + run: PYTHONPATH=toolkit/scripts python3 toolkit/scripts/tests/test_check_entangled_specs.py + + - name: Run spec entanglement checking script run: python3 toolkit/scripts/check_entangled_specs.py . diff --git a/SPECS/fwctl/fwctl.spec b/SPECS/fwctl/fwctl.spec index d031354ddb5..5f40f2a0ec6 100644 --- a/SPECS/fwctl/fwctl.spec +++ b/SPECS/fwctl/fwctl.spec @@ -30,7 +30,7 @@ # SOFTWARE. # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %{!?_name: %define _name fwctl} %{!?_version: %define _version 24.10} diff --git a/SPECS/iser/iser.spec b/SPECS/iser/iser.spec index dfaa7ef13c4..2e4f2205d4a 100644 --- a/SPECS/iser/iser.spec +++ b/SPECS/iser/iser.spec @@ -26,7 +26,7 @@ # # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/isert/isert.spec b/SPECS/isert/isert.spec index 3e7702c0ede..c5993bb1a9c 100644 --- a/SPECS/isert/isert.spec +++ b/SPECS/isert/isert.spec @@ -26,7 +26,7 @@ # # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/knem/knem.spec b/SPECS/knem/knem.spec index 8ae3550e07f..50177448a37 100644 --- a/SPECS/knem/knem.spec +++ b/SPECS/knem/knem.spec @@ -26,7 +26,7 @@ # KMP is disabled by default %{!?KMP: %global KMP 0} -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/mft_kernel/mft_kernel.spec b/SPECS/mft_kernel/mft_kernel.spec index fa678d5fe0d..8ad3036b5d5 100644 --- a/SPECS/mft_kernel/mft_kernel.spec +++ b/SPECS/mft_kernel/mft_kernel.spec @@ -1,4 +1,4 @@ -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/mlnx-nfsrdma/mlnx-nfsrdma.spec b/SPECS/mlnx-nfsrdma/mlnx-nfsrdma.spec index 5c717d4f027..8222294f64c 100644 --- a/SPECS/mlnx-nfsrdma/mlnx-nfsrdma.spec +++ b/SPECS/mlnx-nfsrdma/mlnx-nfsrdma.spec @@ -26,7 +26,7 @@ # # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/mlnx-ofa_kernel/mlnx-ofa_kernel.spec b/SPECS/mlnx-ofa_kernel/mlnx-ofa_kernel.spec index 7061fd48100..9d6c3e3fa36 100644 --- a/SPECS/mlnx-ofa_kernel/mlnx-ofa_kernel.spec +++ b/SPECS/mlnx-ofa_kernel/mlnx-ofa_kernel.spec @@ -25,7 +25,7 @@ # and/or other materials provided with the distribution. # # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/srp/srp.spec b/SPECS/srp/srp.spec index 38786cdd892..bfbb57734eb 100644 --- a/SPECS/srp/srp.spec +++ b/SPECS/srp/srp.spec @@ -26,7 +26,7 @@ # # -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/SPECS/xpmem/xpmem.spec b/SPECS/xpmem/xpmem.spec index 89f8f488233..1d56e366384 100644 --- a/SPECS/xpmem/xpmem.spec +++ b/SPECS/xpmem/xpmem.spec @@ -1,6 +1,6 @@ %{!?KMP: %global KMP 0} -%global last-known-kernel 6.6.82.1 +%global last-known-kernel 6.6.82.1-1 %if 0%{azl} %global target_kernel_version_full %(/bin/rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel-headers) diff --git a/toolkit/scripts/check_entangled_specs.py b/toolkit/scripts/check_entangled_specs.py index b738060a446..1114ce4e511 100755 --- a/toolkit/scripts/check_entangled_specs.py +++ b/toolkit/scripts/check_entangled_specs.py @@ -11,6 +11,10 @@ from pyrpm.spec import replace_macros, Spec +# Control output verbosity, keeping this module global since we do not +# have a containing top-level scope +verbose=False + version_release_matching_groups = [ frozenset([ "SPECS-SIGNED/kernel-signed/kernel-signed.spec", @@ -147,101 +151,59 @@ ]) ] -def check_spec_tags(base_path: str, tags: List[str], groups: List[FrozenSet]) -> Set[FrozenSet]: - """Returns spec sets which violate matching rules for given tags. """ - err_groups = set() - for group in groups: - variants = defaultdict(set) - - for spec_filename in group: - parsed_spec = Spec.from_file(path.join(base_path, spec_filename)) - for tag in tags: - tag_value = get_tag_value(parsed_spec, tag) - variants[tag].add(tag_value) - - for tag in tags: - if len(variants[tag]) > 1: err_groups.add(group) - return err_groups - - -def check_oot_kmodules(base_path: str, tag: str, groups: List[FrozenSet]) -> Set[FrozenSet]: - """Returns OOT kernel modules which violate matching with kernel-headers version. """ - err_groups = set() - - kernel_headers_spec = Spec.from_file(path.join(base_path, "SPECS/kernel-headers/kernel-headers.spec")) - kernel_headers_version = get_tag_value(kernel_headers_spec, 'version') +def print_verbose(message: str): + "Print 'message' to stdout if global variable 'verbose' is true." + if verbose: + print(message) +def check_spec_tags(base_path: str, tags: dict, groups: List[FrozenSet]) -> bool: + """Check if spec set violates matching rules for any of given tags. Return True/False accordingly.""" + has_error = False for group in groups: + print_verbose(f"Processing group: {group}") + spec_tag_map = {tag: {} for tag in tags} for spec_filename in group: parsed_spec = Spec.from_file(path.join(base_path, spec_filename)) - tag_value = get_tag_value(parsed_spec, tag) - if tag_value != kernel_headers_version: - err_groups.add(spec_filename) - return err_groups - -def check_mstflintver_match_groups(base_path: str) -> Set[FrozenSet]: - return check_spec_tags(base_path, ['mstflintver'], mstflintver_matching_groups) - -def check_sdkver_match_groups(base_path: str) -> Set[FrozenSet]: - return check_spec_tags(base_path, ['sdkver'], sdkver_matching_groups) - -def check_version_release_match_groups(base_path: str) -> Set[FrozenSet]: - return check_spec_tags(base_path, ['epoch', 'version', 'release'], version_release_matching_groups) - -def check_version_match_groups(base_path: str) -> Set[FrozenSet]: - return check_spec_tags(base_path, ['epoch', 'version'], version_matching_groups) + print_verbose(f"\t{spec_filename}") -def check_oot_kmodule_matching_groups(base_path: str) -> Set[FrozenSet]: - return check_oot_kmodules(base_path, 'last-known-kernel', oot_kmodule_matching_groups) + for tag, tag_current in tags.items(): + tag_value = get_tag_value(parsed_spec, tag) + spec_tag_map[tag][spec_filename] = tag_value + tag_want = f" (want: {tag_current})" if tag_current else "" + print_verbose(f"\t\ttag({tag}) value: {tag_value}{tag_want}") + + for tag, specs_values in spec_tag_map.items(): + # Skip to next tag if tag value is unique and it matches "tag_expected_value" if set + value_list = list(specs_values.values()) + tag_expected = tags[tag] + if len(set(value_list)) > 1 or (tag_expected and value_list[0] != tag_expected): + has_error = True + print(f'Mismatch in expected value of "{tag}":{tag_expected or ""}') + for spec_name, value in specs_values.items(): + print(f"\t{value:30} => {spec_name}") + + return has_error def check_matches(base_path: str): - version_match_errors = check_version_match_groups(base_path) - version_release_match_errors = check_version_release_match_groups(base_path) - sdkver_match_errors = check_sdkver_match_groups(base_path) - mstflintver_match_errors = check_mstflintver_match_groups(base_path) - oot_kmodule_match_errors = check_oot_kmodule_matching_groups(base_path) - - printer = pprint.PrettyPrinter() - - if len(version_match_errors) or \ - len(version_release_match_errors) or \ - len(sdkver_match_errors) or \ - len(mstflintver_match_errors) or \ - len(oot_kmodule_match_errors): - print('The current repository state violates a spec entanglement rule!') - - if len(version_match_errors): - print( - '\nPlease update the following sets of specs to have the same "Epoch" and "Version" tags:') - for e in version_match_errors: - printer.pprint(e) - - if len(version_release_match_errors): - print( - '\nPlease update the following sets of specs to have the same "Epoch", "Version", and "Release" tags:') - for e in version_release_match_errors: - printer.pprint(e) - - if len(sdkver_match_errors): - print( - '\nPlease update the following sets of specs to have the same "sdkver" global variables:') - for e in sdkver_match_errors: - printer.pprint(e) - - if len(mstflintver_match_errors): - print( - '\nPlease update the following sets of specs to have the same "mstflintver" global variables:') - for e in mstflintver_match_errors: - printer.pprint(e) - - if len(oot_kmodule_match_errors): - print( - '\nPlease update the following sets of specs to match the "last-known-kernel" global variable with kernel-headers "version":') - for e in oot_kmodule_match_errors: - printer.pprint(e) - + kernel_headers_spec = Spec.from_file(path.join(base_path, "SPECS/kernel-headers/kernel-headers.spec")) + kernel_headers_version = get_tag_value(kernel_headers_spec, 'version') + kernel_headers_release = get_tag_value(kernel_headers_spec, 'release') + kernel_version_release = f"{kernel_headers_version}-{kernel_headers_release}" + + groups_to_check = [({'mstflintver':{}}, mstflintver_matching_groups), + ({'sdkver':{}}, sdkver_matching_groups), + ({'epoch':{}, 'version':{}, 'release':{}}, version_release_matching_groups), + ({'epoch':{}, 'version':{}}, version_matching_groups), + ({'last-known-kernel' : kernel_version_release}, oot_kmodule_matching_groups)] + + check_result = [] + for check_args in groups_to_check: + print_verbose(f'Calling check_spec_tags with "{check_args}"') + check_result.append(check_spec_tags(base_path, *check_args)) + if any(check_result): + print('The current repository state violates one or more spec entanglement rule!') sys.exit(1) - + print('Repository state is consistent with spec entanglement rules.') def get_tag_value(spec: "Spec", tag: str) -> str: value = getattr(spec, tag) @@ -249,10 +211,15 @@ def get_tag_value(spec: "Spec", tag: str) -> str: value = replace_macros(value, spec) return value +def main(): + global verbose -if __name__ == '__main__': parser = argparse.ArgumentParser() - parser.add_argument( - 'repo_root', help='path to the root of the Azure Linux repository') + parser.add_argument('repo_root', help='path to the root of the Azure Linux repository') + parser.add_argument ("--verbose", action="store_true", help='Print details about each action') args = parser.parse_args() + verbose = args.verbose check_matches(args.repo_root) + +if __name__ == '__main__': + main() diff --git a/toolkit/scripts/tests/test_check_entangled_specs.py b/toolkit/scripts/tests/test_check_entangled_specs.py new file mode 100644 index 00000000000..a1feb922bdf --- /dev/null +++ b/toolkit/scripts/tests/test_check_entangled_specs.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import unittest +from unittest.mock import patch, MagicMock +import check_entangled_specs + +# Enable verbose output to make debugging easier +check_entangled_specs.verbose = True + +class TestCheckSpecTags(unittest.TestCase): + + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + def test_check_spec_tags_no_errors(self, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["v1", "r1", "v1", "r1", + "v1.1", "r1.1", "v1.1", "r1.1"] + mock_from_file.return_value = MagicMock() + + base_path = "/fake/path" + tags = {"version":{}, "release":{}} + groups = [frozenset(["spec1.spec", "spec2.spec"]), + frozenset(["spec3.spec", "spec4.spec"])] + + result = check_entangled_specs.check_spec_tags(base_path, tags, groups) + self.assertFalse(result) + + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + def test_check_spec_tags_with_errors(self, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["v2", "r2", "v2.1", "r2.1"] + mock_from_file.return_value = MagicMock() + + base_path = "/fake/path" + tags = {"version":"", "release":""} + groups = [frozenset(["spec5.spec", "spec6.spec"])] + + result = check_entangled_specs.check_spec_tags(base_path, tags, groups) + self.assertTrue(result) + + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + def test_check_spec_tags_with_expected_value(self, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["v3","v3"] + mock_from_file.return_value = MagicMock() + + base_path = "/fake/path" + tags = {"version":"v3"} + groups = [frozenset(["spec7.spec", "spec8.spec"])] + + result = check_entangled_specs.check_spec_tags(base_path, tags, groups) + self.assertFalse(result) + + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + def test_check_spec_tags_with_mismatched_expected_value(self, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["v4","v4"] + mock_from_file.return_value = MagicMock() + + base_path = "/fake/path" + tags = {"version":"v5"} + groups = [frozenset(["spec1.spec", "spec2.spec"])] + + result = check_entangled_specs.check_spec_tags(base_path, tags, groups) + self.assertTrue(result) + +class TestCheckMatches(unittest.TestCase): + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + @patch('check_entangled_specs.check_spec_tags') + @patch('sys.exit') + def test_check_matches_no_errors(self, mock_exit, mock_check_spec_tags, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["1.0", "1"] + mock_from_file.return_value = MagicMock() + mock_check_spec_tags.side_effect = [ False, False, False, False, False ] + base_path = "/fake/path" + check_entangled_specs.check_matches(base_path) + mock_exit.assert_not_called() + + @patch('check_entangled_specs.Spec.from_file') + @patch('check_entangled_specs.get_tag_value') + @patch('check_entangled_specs.check_spec_tags') + @patch('sys.exit') + def test_check_matches_with_errors(self, mock_exit, mock_check_spec_tags, mock_get_tag_value, mock_from_file): + mock_get_tag_value.side_effect = ["1.0", "1"] + mock_from_file.return_value = MagicMock() + mock_check_spec_tags.side_effect = [False, False, True, False, False] + base_path = "/fake/path" + check_entangled_specs.check_matches(base_path) + mock_exit.assert_called_once_with(1) + +if __name__ == '__main__': + unittest.main()