diff --git a/test/test_helpers.bzl b/test/test_helpers.bzl index 33bac0c..e28a34d 100644 --- a/test/test_helpers.bzl +++ b/test/test_helpers.bzl @@ -26,6 +26,47 @@ FIXTURE_TAGS = [ "manual", ] +def assert_warning(env, msg_id, data = None): + """Asserts that a warning was printed. + + The logic in this helper is specifically meant to handle warnings printed by + `xcode_config.bzl`, because we want to preserve the behavior of the original + rules and their tests. + + Args: + env: The analysis test environment. + msg_id: The fixed identifier of the warning message. + data: The semicolon-delimited, sorted by key, `key=value` string + representation of the format arguments for the message, if any. + """ + expected = "Warning:{}".format(msg_id) + if data: + expected += ":{}".format(data) + + found_warnings = [] + + actions = analysistest.target_actions(env) + for action in actions: + mnemonic = action.mnemonic + if mnemonic == expected: + return + if mnemonic.startswith("Warning:"): + found_warnings.append(mnemonic) + + found_warnings_msg = "" + if found_warnings: + found_warnings_msg = "; the following warnings were emitted:\n{}".format( + "\n".join(found_warnings), + ) + + analysistest.fail( + env, + "Expected warning '{}' was not emitted{}".format( + expected, + found_warnings_msg, + ), + ) + def find_action(env, mnemonic): """Finds the first action with the given mnemonic in the target under test. diff --git a/test/xcode_config_test.bzl b/test/xcode_config_test.bzl index 884c1d2..fa95908 100644 --- a/test/xcode_config_test.bzl +++ b/test/xcode_config_test.bzl @@ -21,6 +21,7 @@ load( ) load( "@build_bazel_apple_support//xcode:xcode_config.bzl", + "UNAVAILABLE_XCODE_MESSAGE", "xcode_config", ) load( @@ -35,7 +36,13 @@ load( "@build_bazel_apple_support//xcode/private:providers.bzl", "XcodeVersionPropertiesInfo", ) # buildifier: disable=bzl-visibility -load(":test_helpers.bzl", "FIXTURE_TAGS", "find_action", "make_all_tests") +load( + ":test_helpers.bzl", + "FIXTURE_TAGS", + "assert_warning", + "find_action", + "make_all_tests", +) visibility("private") @@ -333,6 +340,12 @@ def _prefers_flag_over_mutually_available_test_impl(ctx): asserts.true(env, "no-local" in xcode_version_info.execution_info()) asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info()) + assert_warning( + env, + "version_not_available_locally", + "command={};local_versions=8.4;version=5.1.2".format(UNAVAILABLE_XCODE_MESSAGE), + ) + return analysistest.end(env) _prefers_flag_over_mutually_available_test = analysistest.make( @@ -369,16 +382,18 @@ def _warn_with_explicit_local_only_version_test_impl(ctx): target_under_test = analysistest.target_under_test(env) xcode_version_info = target_under_test[apple_common.XcodeVersionConfig] - # TODO: b/311385128 - Once we move the rules to apple_support, hack up - # something that would let us actually test the warning messages. We can't - # test `print`. - asserts.equals(env, "8.4", str(xcode_version_info.xcode_version())) asserts.equals(env, "local", xcode_version_info.availability()) asserts.true(env, "requires-darwin" in xcode_version_info.execution_info()) asserts.true(env, "no-remote" in xcode_version_info.execution_info()) asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info()) + assert_warning( + env, + "explicit_version_not_available_remotely", + "version=8.4", + ) + return analysistest.end(env) _warn_with_explicit_local_only_version_test = analysistest.make( @@ -415,16 +430,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_main_version_test_impl( target_under_test = analysistest.target_under_test(env) xcode_version_info = target_under_test[apple_common.XcodeVersionConfig] - # TODO: b/311385128 - Once we move the rules to apple_support, hack up - # something that would let us actually test the warning messages. We can't - # test `print`. - asserts.equals(env, "8.4", str(xcode_version_info.xcode_version())) asserts.equals(env, "local", xcode_version_info.availability()) asserts.true(env, "requires-darwin" in xcode_version_info.execution_info()) asserts.true(env, "no-remote" in xcode_version_info.execution_info()) asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info()) + assert_warning( + env, + "local_default_not_available_remotely", + "local_version=8.4;remote_versions=5.1.2", + ) + return analysistest.end(env) _prefer_local_default_if_no_mutual_no_flag_different_main_version_test = analysistest.make( @@ -460,16 +477,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_build_alias_test_impl(c target_under_test = analysistest.target_under_test(env) xcode_version_info = target_under_test[apple_common.XcodeVersionConfig] - # TODO: b/311385128 - Once we move the rules to apple_support, hack up - # something that would let us actually test the warning messages. We can't - # test `print`. - asserts.equals(env, "10.0.0.10C504", str(xcode_version_info.xcode_version())) asserts.equals(env, "local", xcode_version_info.availability()) asserts.true(env, "requires-darwin" in xcode_version_info.execution_info()) asserts.true(env, "no-remote" in xcode_version_info.execution_info()) asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info()) + assert_warning( + env, + "local_default_not_available_remotely", + "local_version=10.0.0.10C504;remote_versions=10.0", + ) + return analysistest.end(env) _prefer_local_default_if_no_mutual_no_flag_different_build_alias_test = analysistest.make( @@ -505,16 +524,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_full_version_test_impl( target_under_test = analysistest.target_under_test(env) xcode_version_info = target_under_test[apple_common.XcodeVersionConfig] - # TODO: b/311385128 - Once we move the rules to apple_support, hack up - # something that would let us actually test the warning messages. We can't - # test `print`. - asserts.equals(env, "10.0.0.10C504", str(xcode_version_info.xcode_version())) asserts.equals(env, "local", xcode_version_info.availability()) asserts.true(env, "requires-darwin" in xcode_version_info.execution_info()) asserts.true(env, "no-remote" in xcode_version_info.execution_info()) asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info()) + assert_warning( + env, + "local_default_not_available_remotely", + "local_version=10.0.0.10C504;remote_versions=10.0.0.101ff", + ) + return analysistest.end(env) _prefer_local_default_if_no_mutual_no_flag_different_full_version_test = analysistest.make( @@ -554,10 +575,6 @@ def _choose_newest_mutual_xcode_test_impl(ctx): target_under_test = analysistest.target_under_test(env) xcode_version_info = target_under_test[apple_common.XcodeVersionConfig] - # TODO: b/311385128 - Once we move the rules to apple_support, hack up - # something that would let us actually test the warning messages. We can't - # test `print`. - asserts.equals(env, "10", str(xcode_version_info.xcode_version())) asserts.equals(env, "both", xcode_version_info.availability()) asserts.true(env, "requires-darwin" in xcode_version_info.execution_info()) diff --git a/xcode/xcode_config.bzl b/xcode/xcode_config.bzl index 18f155e..bbdb764 100644 --- a/xcode/xcode_config.bzl +++ b/xcode/xcode_config.bzl @@ -23,7 +23,7 @@ load( visibility("public") -unavailable_xcode_message = "'bazel fetch --configure' (Bzlmod) or 'bazel sync --configure' (WORKSPACE)" +UNAVAILABLE_XCODE_MESSAGE = "'bazel fetch --configure' (Bzlmod) or 'bazel sync --configure' (WORKSPACE)" def _xcode_config_impl(ctx): apple_fragment = ctx.fragments.apple @@ -54,6 +54,7 @@ def _xcode_config_impl(ctx): remote_versions, ): xcode_version_properties, availability = _resolve_xcode_from_local_and_remote( + ctx.actions, local_versions, remote_versions, apple_fragment.xcode_version_flag, @@ -208,6 +209,7 @@ def _aliases_to_xcode_version(versions): return version_map def _resolve_xcode_from_local_and_remote( + actions, local_versions, remote_versions, xcode_version_flag, @@ -256,32 +258,29 @@ def _resolve_xcode_from_local_and_remote( ) elif local_version_from_flag: - error = ( - " --xcode_version={} specified, but it is not available remotely. Actions " + - "requiring Xcode will be run locally, which could make your build slower." - ).format( - xcode_version_flag, - ) - if (mutually_available_versions): - error += " Consider using one of [{}].".format( - ", ".join([version for version in mutually_available_versions]), + if mutually_available_versions: + _warn( + actions, + "explicit_version_not_available_remotely_consider_mutual", + version = xcode_version_flag, + mutual_versions = [version for version in mutually_available_versions], + ) + else: + _warn( + actions, + "explicit_version_not_available_remotely", + version = xcode_version_flag, ) - - # buildifier: disable=print - print(error) return local_version_from_flag.xcode_version_properties, "LOCAL" elif remote_version_from_flag: - # buildifier: disable=print - print(("--xcode_version={version} specified, but it is not available locally. " + - "Your build will fail if any actions require a local Xcode. " + - "If you believe you have '{version}' installed, try running {command}," + - "and then re-run your command. Locally available versions: {local_versions}. ") - .format( + _warn( + actions, + "version_not_available_locally", version = xcode_version_flag, - command = unavailable_xcode_message, + command = UNAVAILABLE_XCODE_MESSAGE, local_versions = ", ".join([version for version in local_alias_to_version_map.keys()]), - )) + ) availability = "REMOTE" return remote_version_from_flag.xcode_version_properties, availability @@ -293,7 +292,7 @@ def _resolve_xcode_from_local_and_remote( " you believe you have '{0}' installed, try running {1}, and then" + " re-run your command.").format( xcode_version_flag, - unavailable_xcode_message, + UNAVAILABLE_XCODE_MESSAGE, ", ".join([version.xcode_version_properties.xcode_version for version in local_versions]), ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions]), ), @@ -305,12 +304,11 @@ def _resolve_xcode_from_local_and_remote( # If there aren't any mutually available versions, select the local default. if not mutually_available_versions: - # buildifier: disable=print - print( - ("Using a local Xcode version, '{}', since there are no" + - " remotely available Xcodes on this machine. Consider downloading one of the" + - " remotely available Xcode versions ({}) in order to get the best build" + - " performance.").format(local_default_version.xcode_version_properties.xcode_version, ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions])), + _warn( + actions, + "local_default_not_available_remotely", + local_version = local_default_version.xcode_version_properties.xcode_version, + remote_versions = ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions]), ) local_version = local_default_version availability = "LOCAL" @@ -385,3 +383,57 @@ def _resolve_explicitly_defined_version( def _dotted_version_or_default(field, default): return apple_common.dotted_version(field) or default + +_WARNINGS = { + "version_not_available_locally": """\ +--xcode_version={version} specified, but it is not available locally. \ +Your build will fail if any actions require a local Xcode. \ +If you believe you have '{version}' installed, try running {command}, \ +and then re-run your command. Locally available versions: {local_versions}. +""", + "local_default_not_available_remotely": """\ +Using a local Xcode version, '{local_version}', since there are no \ +remotely available Xcodes on this machine. Consider downloading one of the \ +remotely available Xcode versions ({remote_versions}) in order to get the best \ +build performance. +""", + "explicit_version_not_available_remotely": """\ +--xcode_version={version} specified, but it is not available remotely. Actions \ +requiring Xcode will be run locally, which could make your build slower. +""", + "explicit_version_not_available_remotely_consider_mutual": """\ +--xcode_version={version} specified, but it is not available remotely. Actions \ +requiring Xcode will be run locally, which could make your build slower. \ +Consider using one of [{mutual_versions}]. +""", +} + +def _warn(actions, msg_id, **kwargs): + """Print a warning and also record it as a testable dummy action. + + Starlark doesn't support testing the output of the `print()` function, so + this function also registers a dummy action with a specifically formatted + mnemonic that can be read in the test using the `assert_warning` helper. + + Args: + actions: The object used to register actions. + msg_id: A string identifying the warning as a key in the `_WARNINGS` + dictionary. + **kwargs: Formatting arguments for the message string. + """ + + # buildifier: disable=print + print(_WARNINGS[msg_id].format(**kwargs)) + + mnemonic = "Warning:{}".format(msg_id) + if kwargs: + # Sort the format arguments by key so that they're deterministically + # ordered for tests. + sorted_values = [] + for key in sorted(kwargs.keys()): + sorted_values.append("{}={}".format(key, str(kwargs[key]))) + mnemonic += ":{}".format(";".join(sorted_values)) + + actions.do_nothing( + mnemonic = mnemonic, + )