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 well-known features to the OSX CC toolchain to support configurin… #16734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulherman
Copy link

@paulherman paulherman commented Nov 10, 2022

I've not opened a feature request in bazelbuild/bazel but in bazelbuild/rules_cc as I'm not yet clear around the distinction between the two (bazelbuild/rules_cc#139).

I plan to follow-up with changes to the other toolchains and afterwards to docs as separate PRs.

One thing to note is that not all actions have C counterparts. There is another issue opened (#10411) for that, but tests all pass as things are today.

@paulherman paulherman marked this pull request as ready for review November 11, 2022 11:45
@sgowroji sgowroji added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Nov 11, 2022
@oquenchil
Copy link
Contributor

@googlewalt could you please take a look?

@googlewalt
Copy link
Contributor

@keith Can you take a look to see if this is reasonable?

@googlewalt
Copy link
Contributor

IIUC this implementation requires that user disable std_cxx11 in order to enable a diffferent C++ standard, which seems a little hard to use. Is that what we want?

Another concern is this will hard fail if multiple langauge standard features are specified. This is good for error checking. I think this is probably ok, but I'm not sure if there may be cases where hard fail is undesirable.

A couple other possible ways to implement this:

  • Don't enable any of these features by default. Instead, set up a flag_group that adds the default standard if no language feature is selected. This would be my vote.

  • Don't set the "provides" clause in the features. Order the default feature first, and have it enabled true. This way other features will override the default. This implementation would also not fail if multiple standard features for the same language are specified.

@buildbreaker2021 buildbreaker2021 added awaiting-user-response Awaiting a response from the author awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-review PR is awaiting review from an assigned reviewer labels Feb 10, 2023
@sgowroji sgowroji removed the awaiting-review PR is awaiting review from an assigned reviewer label Feb 24, 2023
@meteorcloudy meteorcloudy requested a review from a team as a code owner November 13, 2023 10:19
@brentleyjones
Copy link
Contributor

brentleyjones commented Nov 13, 2023

Will need to rebase at a minimum, because this toolchain doesn't exist in Bazel anymore, it's now over in https://github.com/bazelbuild/apple_support/tree/master/crosstool. Opening an issue/PR over there, and/or discussing it in the #rules_apple-maintainers-discuss Slack channel would be my recommendation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author platform: apple team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants