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.

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

Jump to bottom

Allow removal of default cc flags #14735

Closed
wants to merge 2 commits into from
Closed

Allow removal of default cc flags #14735

wants to merge 2 commits into from

Conversation

jheaff1
Copy link
Contributor

@jheaff1 jheaff1 commented Feb 6, 2022

This PR modifies the cc toolchain logic so that users can remove (most of) the default compile/linker flags if they so wish. This is required when building 3rd party projects, e.g. Qt, using Bazel.

@jheaff1 jheaff1 requested a review from lberki as a code owner February 6, 2022 16:55
@jheaff1 jheaff1 force-pushed the allow_removal_of_default_cc_flags branch from 55fc190 to 7a979a5 Compare February 7, 2022 10:11
@jheaff1 jheaff1 mentioned this pull request Mar 7, 2022
Closed
@sgowroji sgowroji added the team-Rules-CPP Issues for C++ rules label Mar 24, 2022
@sgowroji sgowroji added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 21, 2022
@jheaff1 jheaff1 mentioned this pull request Jun 4, 2022
Closed
@jheaff1
Starlarkify msvc linker flag logic (bazelbuild#12737)
606ff4c 
Previously, it was not possible to remove "/DEFAULTLIB:msvcrt" from the
linker command line as native logic would disallow disabling the
"dynamic_link_msvcrt" cc "feature".

This would cause build failures in certain scenarios, e.g. when
building the Qt library using Bazel.

By starlarkifying the native logic, users can now remove the
"/DEFAULTLIB" linker flag by adding
"--features=-dynamic_link_msvcrt[_no_debug]" to the invocation of
bazel.
@jheaff1
Allow removal of default cc flags
057059d 
Previously, it was not possible to disable cc "features" such as
"default_compile_flags", which would cause builds to fail in certain
scenarios, such as building 3rd party projects under Bazel.

Now, users can remove default flags (like D_WIN32_WINNT) by adding
"--features=-default_compile_flags" to the invocation of Bazel".
@jheaff1 jheaff1 force-pushed the allow_removal_of_default_cc_flags branch from 7a979a5 to 057059d Compare June 5, 2022 11:06
@jheaff1
Copy link
Contributor Author

jheaff1 commented Jun 5, 2022

Any chance of a review of this PR?

@jheaff1
Copy link
Contributor Author

jheaff1 commented Jun 23, 2022

Friendly ping

@lberki lberki requested review from oquenchil and removed request for lberki June 23, 2022 15:00
@lberki
Copy link
Contributor

lberki commented Jun 23, 2022

Erm, sorry for the delay. Routing this to the right person.

@jheaff1
Copy link
Contributor Author

jheaff1 commented Aug 23, 2022

@oquenchil Friendly ping

oquenchil
oquenchil approved these changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

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

LGTM

@oquenchil oquenchil added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Aug 29, 2022
@copybara-service copybara-service bot closed this in 25d17f5 Sep 1, 2022
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 1, 2022
@jheaff1 jheaff1 mentioned this pull request Sep 19, 2022
Open
aiuto pushed a commit to aiuto/bazel that referenced this pull request Oct 12, 2022
@jheaff1 @aiuto
Allow removal of default cc flags
bf9deb9 
This PR modifies the cc toolchain logic so that users can remove (most of) the default compile/linker flags if they so wish. This is required when building 3rd party projects, e.g. Qt, using Bazel.

Closes bazelbuild#14735.

PiperOrigin-RevId: 471502084
Change-Id: Idbc3e6685bc10c6b51fdff7ab7839dc9e00a5209
@carpenterjc carpenterjc mentioned this pull request Mar 30, 2023
Open
@edevil
Copy link

edevil commented Jul 7, 2023 •
edited

Hey @jheaff1, thank you for fixing this.

Mind providing an example of how one can nowadays disable default_compile_flags, or at least override the _WIN32_WINNT=0x0601 define that Bazel sets by default?

For the record I tried adding features = ["-default_compile_flags"], to my cc_library rule but got the error:

Traceback (most recent call last):
        File "/virtual_builtins_bzl/common/cc/cc_library.bzl", line 32, column 57, in _cc_library_impl
Error in configure_features: The C++ toolchain '@local_config_cc//:toolchain' unconditionally implies feature 'default_compile_flags', which is unsupported by this rule. This is most likely a misconfiguration in the C++ toolchain.

I'm using Bazel 6.2.1.

@jheaff1
Copy link
Contributor Author

jheaff1 commented Jul 7, 2023

I’m surprised that adding features = [“-default_compile_flags”] didn’t work. The behaviour must have changed since my PR was merged

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

@oquenchil oquenchil oquenchil approved these changes

Assignees
No one assigned
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jheaff1 @lberki @edevil @oquenchil @sgowroji