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

Actually prepend copts so they get passed to tools #40

Merged
merged 1 commit into from
Apr 16, 2020

Conversation

segiddins
Copy link
Member

Due to a prior refactor, _prepend_copts didn't actually prepend to
its parameters, but rather re-assigned.

This PR ensures that the list that gets passed through to the underlying
rules is modified.

This change is tested by adding a library that cannot compile without
the copts from its xcconfig applied. ObjC files using manual reference
counting cannot be compiled without the -fobjc-weak flag, which in
this case is only applied by the CLANG_ENABLE_OBJC_WEAK xcconfig.

Due to a prior refactor, `_prepend_copts` didn't actually prepend to
its parameters, but rather re-assigned.

This PR ensures that the list that gets passed through to the underlying
rules is modified.

This change is tested by adding a library that cannot compile without
the `copts` from its xcconfig applied. ObjC files using manual reference
counting cannot be compiled without the `-fobjc-weak` flag, which in
this case is only applied by the `CLANG_ENABLE_OBJC_WEAK` xcconfig.
@segiddins segiddins force-pushed the segiddins/actually-prepend-copts branch from 5d1528b to 1fe8f45 Compare April 15, 2020 17:04
@@ -39,4 +39,12 @@ apple_framework(
},
)

apple_framework(
name = "wont_build_without_xcconfig",
non_arc_srcs = ["weak_property.m"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know for sure that this option is working properly? Could potentially explicitly add a call to addRef and release so that compiling the code double checks that.

Copy link
Member Author

Choose a reason for hiding this comment

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

We do, note that this target fails to compile without the xcconfig applied

@segiddins segiddins merged commit 33021fe into master Apr 16, 2020
@segiddins segiddins deleted the segiddins/actually-prepend-copts branch March 20, 2021 00:34
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