Skip to content

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Apr 16, 2022

By specifying SYSTEM_HEADER_SEARCH_PATHS in Xcode build settings.

Partially addresses #82.

Allows Yams to compile.

By specifying `HEADER_SEARCH_PATHS` in Xcode build settings.

Partially addresses #82.

Allows [Yams](https://github.com/jpsim/Yams) to compile.
@brentleyjones
Copy link
Contributor

brentleyjones commented Apr 16, 2022

This should be handled by the other mechanisms that collect header include paths. Ideally we don't look at ctx.attr if possible, to allow custom rules to "just work".

Can you debug a bit to see where the disconnect is? We look at CcInfo.compilation_context, which should have these, in _process_search_paths(). Of note, we need to set search paths there to allow generator to transform bazel-out/ and external/ paths.

@brentleyjones
Copy link
Contributor

Looks like cc_library sets these in CcInfo.compilation_context.system_includes. I thought that was going to be for SDK paths, but thankfully it's not.

So, the more complete fix is to parse that in _process_search_paths() and set it to SYSTEM_HEADER_SEARCH_PATHS ingenerator. Also, we should parse -isystem in opts.bzl similar to -iquote and have opts_search_paths have a system_includes.

@jpsim
Copy link
Contributor Author

jpsim commented Apr 17, 2022

Thanks for the guidance (again!). I applied what I think you asked for, and confirmed that Yams is still compiling in Xcode with the changes. Let me know if I misinterpreted something.

@jpsim jpsim marked this pull request as ready for review April 17, 2022 20:09
Copy link
Contributor

@brentleyjones brentleyjones left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks!

@brentleyjones brentleyjones merged commit 4fa98de into main Apr 17, 2022
@brentleyjones brentleyjones deleted the support-includes-in-cc_library branch April 17, 2022 23:56
@brentleyjones brentleyjones mentioned this pull request Apr 17, 2022
10 tasks
jpsim added a commit that referenced this pull request Apr 19, 2022
brentleyjones pushed a commit that referenced this pull request Apr 19, 2022
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