From 946cd9f1358cb5d0223561db7bcc4aa3734ef0ff Mon Sep 17 00:00:00 2001 From: Samy Lanka Date: Mon, 21 Aug 2023 17:45:12 +0000 Subject: [PATCH] SERVER-79361 Make the IDL compatibility checker script stop parsing feature flags in IDL files --- buildscripts/idl/idl/parser.py | 20 ++++--- buildscripts/idl/idl_check_compatibility.py | 14 +++-- etc/evergreen.yml | 28 +++------- .../variants/classic_engine.yml | 4 +- .../variants/misc_release.yml | 12 +--- evergreen/check_idl_compat.sh | 55 +++++++++---------- 6 files changed, 59 insertions(+), 74 deletions(-) diff --git a/buildscripts/idl/idl/parser.py b/buildscripts/idl/idl/parser.py index 41b73966d26..2838b6baaba 100644 --- a/buildscripts/idl/idl/parser.py +++ b/buildscripts/idl/idl/parser.py @@ -1059,11 +1059,13 @@ def _propagate_globals(spec): idltype.cpp_type = _prefix_with_namespace(cpp_namespace, idltype.cpp_type) -def parse_file(stream, error_file_name): - # type: (Any, str) -> syntax.IDLParsedSpec +def parse_file(stream, error_file_name, parse_feature_flags=True): + # type: (Any, str, bool) -> syntax.IDLParsedSpec """ Parse a YAML document into an idl.syntax tree. + If parse_feature_flags is False, don't attempt to parse the feature flag type. + stream: is a io.Stream. error_file_name: just a file name for error messages to use. """ @@ -1116,8 +1118,10 @@ def parse_file(stream, error_file_name): _parse_mapping(ctxt, spec, second_node, "server_parameters", _parse_server_parameter) elif first_name == "configs": _parse_mapping(ctxt, spec, second_node, "configs", _parse_config_option) - elif first_name == "feature_flags": + elif parse_feature_flags and first_name == "feature_flags": _parse_mapping(ctxt, spec, second_node, "feature_flags", _parse_feature_flag) + elif not parse_feature_flags and first_name == "feature_flags": + continue else: ctxt.add_unknown_root_node_error(first_node) @@ -1152,16 +1156,18 @@ class ImportResolverBase(object, metaclass=ABCMeta): pass -def parse(stream, input_file_name, resolver): - # type: (Any, str, ImportResolverBase) -> syntax.IDLParsedSpec +def parse(stream, input_file_name, resolver, parse_feature_flags=True): + # type: (Any, str, ImportResolverBase, bool) -> syntax.IDLParsedSpec """ Parse a YAML document into an idl.syntax tree. + If parse_feature_flags is False, don't attempt to parse the feature flag type. + stream: is a io.Stream. input_file_name: a file name for error messages to use, and to help resolve imported files. """ - root_doc = parse_file(stream, input_file_name) + root_doc = parse_file(stream, input_file_name, parse_feature_flags) if root_doc.errors: return root_doc @@ -1198,7 +1204,7 @@ def parse(stream, input_file_name, resolver): # Parse imported file with resolver.open(resolved_file_name) as file_stream: - parsed_doc = parse_file(file_stream, resolved_file_name) + parsed_doc = parse_file(file_stream, resolved_file_name, parse_feature_flags) # Check for errors if parsed_doc.errors: diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py index 76ce7de7475..0a1d8fe01a5 100644 --- a/buildscripts/idl/idl_check_compatibility.py +++ b/buildscripts/idl/idl_check_compatibility.py @@ -422,7 +422,7 @@ def get_new_commands( with open(new_idl_file_path) as new_file: new_idl_file = parser.parse( new_file, new_idl_file_path, - CompilerImportResolver(import_directories + [new_idl_dir])) + CompilerImportResolver(import_directories + [new_idl_dir]), False) if new_idl_file.errors: new_idl_file.errors.dump_errors() raise ValueError(f"Cannot parse {new_idl_file_path}") @@ -1310,9 +1310,11 @@ def check_error_reply(old_basic_types_path: str, new_basic_types_path: str, ctxt = IDLCompatibilityContext(old_idl_dir, new_idl_dir, IDLCompatibilityErrorCollection()) with open(old_basic_types_path) as old_file: old_idl_file = parser.parse(old_file, old_basic_types_path, - CompilerImportResolver(old_import_directories)) + CompilerImportResolver(old_import_directories), False) if old_idl_file.errors: old_idl_file.errors.dump_errors() + # If parsing old IDL files fails, it might be because the parser has been recently + # updated to require something that isn't present in older IDL files. raise ValueError(f"Cannot parse {old_basic_types_path}") old_error_reply_struct = old_idl_file.spec.symbols.get_struct("ErrorReply") @@ -1322,7 +1324,7 @@ def check_error_reply(old_basic_types_path: str, new_basic_types_path: str, else: with open(new_basic_types_path) as new_file: new_idl_file = parser.parse(new_file, new_basic_types_path, - CompilerImportResolver(new_import_directories)) + CompilerImportResolver(new_import_directories), False) if new_idl_file.errors: new_idl_file.errors.dump_errors() raise ValueError(f"Cannot parse {new_basic_types_path}") @@ -1490,9 +1492,11 @@ def check_compatibility(old_idl_dir: str, new_idl_dir: str, old_import_directori with open(old_idl_file_path) as old_file: old_idl_file = parser.parse( old_file, old_idl_file_path, - CompilerImportResolver(old_import_directories + [old_idl_dir])) + CompilerImportResolver(old_import_directories + [old_idl_dir]), False) if old_idl_file.errors: old_idl_file.errors.dump_errors() + # If parsing old IDL files fails, it might be because the parser has been + # recently updated to require something that isn't present in older IDL files. raise ValueError(f"Cannot parse {old_idl_file_path}") for old_cmd in old_idl_file.spec.symbols.commands: @@ -1559,7 +1563,7 @@ def get_generic_arguments(gen_args_file_path: str, includes: str) -> Tuple[Set[s with open(gen_args_file_path) as gen_args_file: parsed_idl_file = parser.parse(gen_args_file, gen_args_file_path, - CompilerImportResolver(includes)) + CompilerImportResolver(includes), False) if parsed_idl_file.errors: parsed_idl_file.errors.dump_errors() raise ValueError(f"Cannot parse {gen_args_file_path}") diff --git a/etc/evergreen.yml b/etc/evergreen.yml index 76f3acb8451..2cd1f30c336 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -1065,9 +1065,7 @@ buildvariants: - name: compile_integration_and_test_parallel_stream_TG distros: - rhel80-large - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: .aggfuzzer !.feature_flag_guarded !.no_debug_mode - name: .aggregation !.feature_flag_guarded !.no_debug_mode - name: audit @@ -1197,9 +1195,7 @@ buildvariants: - name: analyze_shard_key_jscore_passthrough_gen - name: query_golden_classic - name: lint_fuzzer_sanity_patch - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility # - name: burn_in_tasks_gen # depends_on: # - name: version_burn_in_gen @@ -1427,9 +1423,7 @@ buildvariants: - name: secondary_reads_passthrough_gen - name: session_jscore_passthrough - name: sharding_api_version_jscore_passthrough_gen - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: unittest_shell_hang_analyzer_gen - name: vector_search - name: vector_search_auth @@ -1460,9 +1454,7 @@ buildvariants: - name: analyze_shard_key_jscore_passthrough_gen - name: .cqf - name: lint_fuzzer_sanity_patch - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility # - name: burn_in_tasks_gen # depends_on: # - name: version_burn_in_gen @@ -1962,9 +1954,7 @@ buildvariants: - name: analyze_shard_key_jscore_passthrough_gen - name: query_golden_classic - name: lint_fuzzer_sanity_patch - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: check_feature_flag_tags - name: check_for_todos - name: .aggfuzzer !.cqf_only @@ -2871,9 +2861,7 @@ buildvariants: distros: - amazon2-arm64-large - name: .lint - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: validate_commit_message - name: lint_large_files_check - name: check_feature_flag_tags @@ -2927,9 +2915,7 @@ buildvariants: - name: analyze_shard_key_jscore_passthrough_gen - name: query_golden_classic - name: lint_fuzzer_sanity_patch - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: check_feature_flag_tags - name: check_for_todos - name: .aggfuzzer !.cqf_only !.feature_flag_guarded diff --git a/etc/evergreen_yml_components/variants/classic_engine.yml b/etc/evergreen_yml_components/variants/classic_engine.yml index a9a43c5b96f..d0d39b61c07 100644 --- a/etc/evergreen_yml_components/variants/classic_engine.yml +++ b/etc/evergreen_yml_components/variants/classic_engine.yml @@ -166,9 +166,7 @@ buildvariants: - name: secondary_reads_passthrough_gen - name: session_jscore_passthrough - name: sharding_api_version_jscore_passthrough_gen - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: unittest_shell_hang_analyzer_gen - name: vector_search - name: vector_search_auth diff --git a/etc/evergreen_yml_components/variants/misc_release.yml b/etc/evergreen_yml_components/variants/misc_release.yml index ca4cd1212f7..be4efb1dd12 100644 --- a/etc/evergreen_yml_components/variants/misc_release.yml +++ b/etc/evergreen_yml_components/variants/misc_release.yml @@ -225,9 +225,7 @@ buildvariants: - name: compile_integration_and_test_parallel_stream_TG distros: - amazon2-arm64-large - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: .aggfuzzer !.feature_flag_guarded - name: .aggregation !.feature_flag_guarded - name: audit @@ -387,9 +385,7 @@ buildvariants: - name: compile_test_and_package_serial_no_unittests_TG distros: - amazon2023.0-large - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: .aggfuzzer !.feature_flag_guarded !.multiversion - name: .aggregation !.feature_flag_guarded - name: audit @@ -547,9 +543,7 @@ buildvariants: - name: compile_test_and_package_serial_no_unittests_TG distros: - amazon2023.0-arm64-large - # TODO (SERVER-79361): Re-enable the IDL compatibility checker once the - # IDL compiler issue has been resolved. - # - name: test_api_version_compatibility + - name: test_api_version_compatibility - name: .aggfuzzer !.feature_flag_guarded !.multiversion - name: .aggregation !.feature_flag_guarded - name: audit diff --git a/evergreen/check_idl_compat.sh b/evergreen/check_idl_compat.sh index cfb83ccd6b1..5566cf23dbd 100755 --- a/evergreen/check_idl_compat.sh +++ b/evergreen/check_idl_compat.sh @@ -7,34 +7,31 @@ set -o errexit set -o verbose activate_venv -# TODO (SERVER-79361): Re-enable this code once the IDL compiler issue has been resolved -# for the IDL compatibility script. +$python buildscripts/idl/check_stable_api_commands_have_idl_definitions.py -v --install-dir dist-test/bin --include src --include src/mongo/db/modules/enterprise/src 1 +$python buildscripts/idl/checkout_idl_files_from_past_releases.py -v idls -# $python buildscripts/idl/check_stable_api_commands_have_idl_definitions.py -v --install-dir dist-test/bin --include src --include src/mongo/db/modules/enterprise/src 1 -# $python buildscripts/idl/checkout_idl_files_from_past_releases.py -v idls +function run_idl_check_compatibility { + dir=$1 + output=$( + python buildscripts/idl/idl_check_compatibility.py -v \ + --old-include "$dir/src" \ + --old-include "$dir/src/mongo/db/modules/enterprise/src" \ + --new-include src \ + --new-include src/mongo/db/modules/enterprise/src \ + "$dir/src" src + ) + exit_code=$? + echo "Performing idl check compatibility with release: $dir:" + echo "$output" + if [ $exit_code -ne 0 ]; then + exit 255 + fi +} +export -f run_idl_check_compatibility +find idls -maxdepth 1 -mindepth 1 -type d | xargs -n 1 -P 0 -I % bash -c 'run_idl_check_compatibility "$@"' _ % -# function run_idl_check_compatibility { -# dir=$1 -# output=$( -# python buildscripts/idl/idl_check_compatibility.py -v \ -# --old-include "$dir/src" \ -# --old-include "$dir/src/mongo/db/modules/enterprise/src" \ -# --new-include src \ -# --new-include src/mongo/db/modules/enterprise/src \ -# "$dir/src" src -# ) -# exit_code=$? -# echo "Performing idl check compatibility with release: $dir:" -# echo "$output" -# if [ $exit_code -ne 0 ]; then -# exit 255 -# fi -# } -# export -f run_idl_check_compatibility -# find idls -maxdepth 1 -mindepth 1 -type d | xargs -n 1 -P 0 -I % bash -c 'run_idl_check_compatibility "$@"' _ % - -# # Run the idl compatibility checker script with the current src directory as both the "old" and -# # "new" versions. This is so that we can check that commands that were newly added since the last -# # release adhere to compatibility requirements. -# echo "Performing idl check compatibility for newly added commands since the last release:" -# $python buildscripts/idl/idl_check_compatibility.py -v --old-include src --old-include src/mongo/db/modules/enterprise/src --new-include src --new-include src/mongo/db/modules/enterprise/src src src +# Run the idl compatibility checker script with the current src directory as both the "old" and +# "new" versions. This is so that we can check that commands that were newly added since the last +# release adhere to compatibility requirements. +echo "Performing idl check compatibility for newly added commands since the last release:" +$python buildscripts/idl/idl_check_compatibility.py -v --old-include src --old-include src/mongo/db/modules/enterprise/src --new-include src --new-include src/mongo/db/modules/enterprise/src src src