Skip to content

Only build objects when supports_start_end_lib is enabled #493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Only build objects when supports_start_end_lib is enabled #493

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58cfacf
Only build objects when supports_start_end_lib is enabled
keith Sep 22, 2025
0628e04
output groups
keith Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions cc/private/rules_impl/cc_library.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,14 +238,25 @@ def _cc_library_impl(ctx):
libraries = libraries_to_link,
)

supports_start_end_lib = cc_common.is_enabled(
feature_configuration = feature_configuration,
feature_name = "supports_start_end_lib",
)

files_builder = []
if linking_outputs.library_to_link != None:
artifacts_to_build = linking_outputs.library_to_link
if artifacts_to_build.static_library != None:
files_builder.append(artifacts_to_build.static_library)
if supports_start_end_lib and (artifacts_to_build.objects or artifacts_to_build.pic_objects):
if artifacts_to_build.pic_objects:
files_builder.extend(artifacts_to_build.pic_objects)
Copy link
Collaborator

Choose a reason for hiding this comment

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

BES messages can get larger as they no longer announce just a constant number of outputs per cc_library. Just something to keep in mind, but I think that the change is still worth it.

elif artifacts_to_build.objects:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously both the pic and non-pic version were built, now only one of the two flavors is requested. There are pros and cons to both approaches, I guess. We should at least leave a comment explaining why this differs between the two feature states.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was wondering why that was the case, or in what configurations you would end up with both, but I can play it safe and mirror today's behavior 🤔 , not sure what the ideal is?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would say it's safer if there are fewer outputs (assuming we change the type of outputs anyway) as this results in fewer downloads with default BwoB settings and smaller BES messages. Building pic of available and non-pic only as a fallback seems reasonable to me. I wouldn't expect non-pic compilation to fail if pic compilation passes.

files_builder.extend(artifacts_to_build.objects)
else:
if artifacts_to_build.static_library != None:
files_builder.append(artifacts_to_build.static_library)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could advertise new static_library and pic_static_library output groups so that users can still get these artifacts.

Copy link
Member Author

Choose a reason for hiding this comment

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

added that for discussion, seems fine


if artifacts_to_build.pic_static_library != None:
files_builder.append(artifacts_to_build.pic_static_library)
if artifacts_to_build.pic_static_library != None:
files_builder.append(artifacts_to_build.pic_static_library)

if not cc_common.is_enabled(
feature_configuration = feature_configuration,
Expand Down