Skip to content

Commit

Permalink
Use separate field for artifact identity
Browse files Browse the repository at this point in the history
Summary: Apple prelude implements selective debugging that [requires `Label`](https://fburl.com/code/j484n17w) to be used. For that sake using separate identity field for deduplication purpose.

Reviewed By: milend

Differential Revision: D67395899

fbshipit-source-id: 4f4b9571ccff24cf6e9e93c40ed3ac0acee718f6
  • Loading branch information
Nikita Patskov authored and facebook-github-bot committed Dec 19, 2024
1 parent 736bff0 commit 2b5a01c
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 16 deletions.
6 changes: 3 additions & 3 deletions apple/validation/debug_artifacts.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def get_debug_artifacts_validators(ctx, artifacts: ArtifactTSet) -> dict[str, Ar

return name_to_validation_result

def _get_analysis_input_artifacts(ctx, artifacts: ArtifactTSet) -> dict[Label | str, list[_AnalysisInput]]:
def _get_analysis_input_artifacts(ctx, artifacts: ArtifactTSet) -> dict[Label, list[_AnalysisInput]]:
underlying_tset = artifacts._tset
if underlying_tset == None:
return {}
Expand All @@ -52,7 +52,7 @@ def _analyze_artifacts(
ctx,
key: str,
analysis_tool: RunInfo,
label_to_artifacts: dict[Label | str, list[_AnalysisInput]]) -> dict[Label | str, list[Artifact]]:
label_to_artifacts: dict[Label, list[_AnalysisInput]]) -> dict[Label, list[Artifact]]:
label_to_analysis = {}
for label, inputs in label_to_artifacts.items():
for input in inputs:
Expand All @@ -76,7 +76,7 @@ def _reduce_analysis_artifacts(
ctx,
key: str,
reducer_tool: RunInfo,
label_to_artifacts: dict[Label | str, list[Artifact]]) -> Artifact:
label_to_artifacts: dict[Label, list[Artifact]]) -> Artifact:
input_json = ctx.actions.write_json(
"{}_reducer_args.json".format(key),
label_to_artifacts,
Expand Down
11 changes: 7 additions & 4 deletions artifact_tset.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ ArtifactInfoTag = enum(
)

ArtifactInfo = record(
label = field(Label | str),
label = field(Label),
identity = field(Label | str),
artifacts = field(list[Artifact]),
tags = field(list[ArtifactInfoTag]),
)
Expand All @@ -37,7 +38,7 @@ def _get_identified_artifacts(entries: list[ArtifactInfo]) -> cmd_args:
args = cmd_args()
for entry in entries:
for artifact in entry.artifacts:
format_str = "identified'{}'=".format(stringify_artifact_label(entry.label))
format_str = "identified'{}'=".format(stringify_artifact_label(entry.identity))
args.add(cmd_args(artifact, format = format_str + "{}"))
return args

Expand All @@ -55,7 +56,8 @@ ArtifactTSet = record(
def make_artifact_tset(
actions: AnalysisActions,
# Must be non-`None` if artifacts are passed in to `artifacts`.
label: Label | str | None = None,
label: Label | None = None,
identity: Label | str | None = None,
artifacts: list[Artifact] = [],
infos: list[ArtifactInfo] = [],
children: list[ArtifactTSet] = [],
Expand All @@ -71,7 +73,8 @@ def make_artifact_tset(
# Build list of all non-child values.
values = []
if artifacts:
values.append(ArtifactInfo(label = label, artifacts = artifacts, tags = tags))
artifact_identity = identity if identity else label
values.append(ArtifactInfo(label = label, identity = artifact_identity, artifacts = artifacts, tags = tags))
values.extend(infos)

# If there's no children or artifacts, return `None`.
Expand Down
7 changes: 4 additions & 3 deletions cxx/anon_link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def _serialize_link_info(info: LinkInfo):
info.post_flags,
[_serialize_linkable(linkable) for linkable in info.linkables],
# TODO(agallagher): It appears anon-targets don't allow passing in `label`.
[(stringify_artifact_label(info.label), info.artifacts) for info in external_debug_info],
[(stringify_artifact_label(info.label), stringify_artifact_label(info.identity), info.artifacts) for info in external_debug_info],
[m.version for m in info.metadata],
)

Expand Down Expand Up @@ -145,8 +145,8 @@ def _deserialize_link_info(actions: AnalysisActions, label: Label, info) -> Link
external_debug_info = make_artifact_tset(
actions = actions,
infos = [
ArtifactInfo(label = label, artifacts = artifacts, tags = [])
for _label, artifacts in external_debug_info
ArtifactInfo(label = label, identity = identity, artifacts = artifacts, tags = [])
for _label, identity, artifacts in external_debug_info
],
),
metadata = [DepMetadata(version = v) for v in metadata],
Expand Down Expand Up @@ -242,6 +242,7 @@ ANON_ATTRS = {
# TODO(agallagher): It appears anon-targets don't
# allow passing in `label`.
attrs.string(), # label
attrs.string(), # identity
attrs.list(attrs.source()), # artifacts
),
),
Expand Down
15 changes: 10 additions & 5 deletions cxx/cxx_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,8 @@ def cxx_library_parameterized(ctx: AnalysisContext, impl_params: CxxRuleConstruc
)],
external_debug_info = make_artifact_tset(
actions = ctx.actions,
label = debug_info_identity,
label = ctx.label,
identity = debug_info_identity,
artifacts = (
compiled_srcs.pic.external_debug_info +
(compiled_srcs.pic.objects if compiled_srcs.pic.objects_have_external_debug_info else [])
Expand Down Expand Up @@ -1227,7 +1228,8 @@ def _form_library_outputs(
objects_have_external_debug_info = compiled_srcs.pic_optimized.objects_have_external_debug_info,
external_debug_info = make_artifact_tset(
ctx.actions,
label = debug_info_identity,
label = ctx.label,
identity = debug_info_identity,
artifacts = compiled_srcs.pic_optimized.external_debug_info,
children = impl_params.additional.static_external_debug_info,
),
Expand All @@ -1248,7 +1250,8 @@ def _form_library_outputs(
objects_have_external_debug_info = lib_compile_output.objects_have_external_debug_info,
external_debug_info = make_artifact_tset(
ctx.actions,
label = debug_info_identity,
label = ctx.label,
identity = debug_info_identity,
artifacts = lib_compile_output.external_debug_info,
children = impl_params.additional.static_external_debug_info,
),
Expand Down Expand Up @@ -1289,7 +1292,8 @@ def _form_library_outputs(
external_debug_artifacts.extend(impl_params.extra_link_input)
external_debug_info = make_artifact_tset(
actions = ctx.actions,
label = debug_info_identity,
label = ctx.label,
identity = debug_info_identity,
artifacts = external_debug_artifacts,
children = impl_params.additional.shared_external_debug_info,
tags = impl_params.additional.external_debug_info_tags,
Expand Down Expand Up @@ -1601,7 +1605,8 @@ def _static_library(

all_external_debug_info = make_artifact_tset(
actions = ctx.actions,
label = debug_info_identity,
label = ctx.label,
identity = debug_info_identity,
artifacts = object_external_debug_info,
children = [external_debug_info],
tags = impl_params.additional.external_debug_info_tags,
Expand Down
3 changes: 2 additions & 1 deletion linking/link_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,8 @@ def get_link_args_for_strategy(
)
external_debug_info = make_artifact_tset(
actions = ctx.actions,
label = get_debug_info_identity(ctx),
label = ctx.label,
identity = get_debug_info_identity(ctx),
children = filter(
None,
[x._external_debug_info.get(link_strategy) for x in deps_merged_link_infos] + ([additional_link_info.external_debug_info] if additional_link_info else []),
Expand Down

0 comments on commit 2b5a01c

Please sign in to comment.