Skip to content

Commit

Permalink
Record warnings from the Xcode rules as structured information in dum…
Browse files Browse the repository at this point in the history 
…my actions so that we can test them.

PiperOrigin-RevId: 631862483
  • Loading branch information
@allevato @swiple-rules-gardener
allevato authored and swiple-rules-gardener committed May 8, 2024
1 parent f1bbadc commit 0f6c6a3
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 49 deletions.
41 changes: 41 additions & 0 deletions test/test_helpers.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
59 changes: 38 additions & 21 deletions test/xcode_config_test.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ load(
)
load(
"@build_bazel_apple_support//xcode:xcode_config.bzl",
"UNAVAILABLE_XCODE_MESSAGE",
"xcode_config",
)
load(
Expand All @@ -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")

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
Expand Down
108 changes: 80 additions & 28 deletions xcode/xcode_config.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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]),
),
Expand All @@ -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"
Expand Down Expand Up @@ -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,
)

1 comment on commit 0f6c6a3

@brentleyjones
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#333

Please sign in to comment.