Skip to content
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.

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

Add feature definition for static_link_cpp_runtimes in unix cc toolchain config #17391

Closed
wants to merge 2 commits into from
Closed

Add feature definition for static_link_cpp_runtimes in unix cc toolchain config #17391

wants to merge 2 commits into from

Conversation

yuzhy8701
Copy link
Contributor

static_link_cpp_runtimes is one of the well-known features - which is required if you want to link in non-default toolchain libs. However the default unix toolchain does not define it at all.

This PR adds a simple definition of this feature. The added feature is disabled by default to avoid negative impacts, and can be enabled on demand.

The added feature is disabled by default to avoid negative impacts, and can be enabled on demand.
@yuzhy8701 yuzhy8701 marked this pull request as ready for review February 2, 2023 06:30
@oquenchil oquenchil added team-Rules-CPP Issues for C++ rules awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Feb 2, 2023
@oquenchil oquenchil self-assigned this Feb 2, 2023
@sgowroji
Copy link
Member

sgowroji commented Feb 3, 2023

Hi @yuzhy8701, Could you please fix the above buildkite checks. As the PR is awaiting to merge. Thanks!

@yuzhy8701
Copy link
Contributor Author

The test failures are complaining not finding python:

AssertionError: Could not find python binary: python

I don't think they are related to my change - more like an infra issue.

I can rebase the changes and try again.

@kishorenc
Copy link

Will this PR fix #14342?

@yuzhy8701
Copy link
Contributor Author

Will this PR fix #14342?

No. This PR only defines the feature, but does not add the -static-libstdc++ flag. So users would still need to define flags in the linkopts as if the feature is not enabled. Actually, I think this feature should be renamed, since all it provides by default is linking the libs you provide in static|dynamic_runtime_libs.

@copybara-service copybara-service bot closed this in 6c7147b Feb 7, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Feb 7, 2023
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
…chain config

`static_link_cpp_runtimes` is one of the well-known features - which is required if you want to link in non-default toolchain libs. However the default unix toolchain does not define it at all.

This PR adds a simple definition of this feature. The added feature is disabled by default to avoid negative impacts, and can be enabled on demand.

Closes #17391.

PiperOrigin-RevId: 507677235
Change-Id: If149a50c1ab41dad258f706bc83df0fc09f8e6e7
Synss added a commit to Synss/bazel that referenced this pull request Aug 3, 2024
This patch implements the `static_link_cpp_runtimes` feature
in `unix_cc_toolchain_config`.  The feature is documented as
a well-known feature[1] and was added to the toolchain[2] but
not implemented.

The patch follows the solution proposed on GitHub.[3]

For the feature to work as expected, users have to remove
the linker flag `"-lstdc++"` from `link_libs` (or `link_flags`)
from the call to `unix_cc_toolchain_config`.

[1] https://bazel.build/docs/cc-toolchain-config-reference#wellknown-features
[2] bazelbuild#17391
[3] bazelbuild#14342
Synss added a commit to Synss/bazel that referenced this pull request Aug 3, 2024
This patch implements the `static_link_cpp_runtimes` feature
in `unix_cc_toolchain_config`.  The feature is documented as
a well-known feature[1] and was added to the toolchain[2] but
not implemented.

The patch adapts a solution proposed on GitHub.[3]

[1] https://bazel.build/docs/cc-toolchain-config-reference#wellknown-features
[2] bazelbuild#17391
[3] bazelbuild#14342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants