Skip to content

Starlarkify declare_compile_output_file and declare_other_output_file#758

Open
dzbarsky wants to merge 2 commits into
bazelbuild:mainfrom
dzbarsky:codex/starlarkify-declare-compile-output-file
Open

Starlarkify declare_compile_output_file and declare_other_output_file#758
dzbarsky wants to merge 2 commits into
bazelbuild:mainfrom
dzbarsky:codex/starlarkify-declare-compile-output-file

Conversation

@dzbarsky

@dzbarsky dzbarsky commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace _cc_internal.declare_compile_output_file and _cc_internal.declare_other_output_file with Starlark output declarations. Compile outputs are declared as shareable artifacts at <package>/_objs/<label.name>/<output_name>, while .dwo and ThinLTO indexing outputs are declared beside their object files with actions.declare_file(..., sibling = object_file).

The rule context already provides the configuration used by every caller, so this also removes redundant configuration arguments from _get_compile_output_file, _maybe_declare_dotd_file, _maybe_declare_diagnostics_file, _maybe_declare_gcno_file, and their callers.

Declare compile outputs as shareable artifacts under the package _objs directory using the rule context. Remove the redundant configuration arguments from _get_compile_output_file and its callers.
@dzbarsky dzbarsky changed the title [codex] Starlarkify declare_compile_output_file Starlarkify declare_compile_output_file Jun 19, 2026
@dzbarsky dzbarsky marked this pull request as ready for review June 19, 2026 00:34
Declare per-object debug and ThinLTO indexing outputs beside the object file with actions.declare_file. This preserves the object file parent directory without calling the native helper.
@dzbarsky dzbarsky changed the title Starlarkify declare_compile_output_file [codex] Starlarkify declare_compile_output_file and declare_other_output_file Jun 19, 2026
@dzbarsky dzbarsky changed the title [codex] Starlarkify declare_compile_output_file and declare_other_output_file Starlarkify declare_compile_output_file and declare_other_output_file Jun 19, 2026

@lilygorsheneva lilygorsheneva left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I ran into LTO issues when trying to change declare_other_output_file; curious to see if this will run into the same issues; approving for internal testing.

@lilygorsheneva

Copy link
Copy Markdown
Collaborator

I ran into LTO issues when trying to change declare_other_output_file; curious to see if this will run into the same issues; approving for internal testing.

Internal testing shows "CppLTOIndexing failed post-execution validation" errors; I'll see if I can reproduce it in the LTO tests in this repo.

Failure in LTO tests migrated from Java (merged today) https://buildkite.com/bazel/google-rules-cc-presubmit/builds/2539/canvas?jid=019f14e4-33c3-49b2-ba62-84ca63afcff2&tab=output

@dzbarsky

Copy link
Copy Markdown
Contributor Author

I ran into LTO issues when trying to change declare_other_output_file; curious to see if this will run into the same issues; approving for internal testing.

Internal testing shows "CppLTOIndexing failed post-execution validation" errors; I'll see if I can reproduce it in the LTO tests in this repo.

Failure in LTO tests migrated from Java (merged today) https://buildkite.com/bazel/google-rules-cc-presubmit/builds/2539/canvas?jid=019f14e4-33c3-49b2-ba62-84ca63afcff2&tab=output

cool i'll rebase and take a look

@lilygorsheneva

Copy link
Copy Markdown
Collaborator

I'm not yet sure why tests/integration/lto_tests.sh isn't catching these validation errors.

@lilygorsheneva

Copy link
Copy Markdown
Collaborator

Looks like the validation issues all happen in paths that nest LTO and _objs subdirectories (via a rule I haven't found an external equivalent of yet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants