Removing the C_INCLUDE_DIRS feature.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Technically there is some risk but I think the blast radius, if present, is extremely small.
C_INCLUDE_DIRS is not recognized by GCC.
If I use https://sourcegraph.com/search?q=context:global+%5CbC_INCLUDE_DIRS%3D+-f:clang/&patternType=regexp&case=yes&sm=1&groupBy=repo to find the use cases, one project has a use like add_opt C_INCLUDE_DIRS=$C_INCLUDES, but the search patch seems likely redundant.
Just FYI I am not in a rush to commit this. I am posting this more so as a means of prodding for discussion of the feature.
So far nobody has popped up to say they want it.
@MaskRay I poked around a bit on sourcegraph.com and didn't see any statement about what it actually searches, other than vague "all your repositories" and "all your code." The "your" bit makes me wonder how broad it is really. The website doesn't say.
They index quite a lot: https://about.sourcegraph.com/blog/indexing-the-oss-universe-update-more-code-hosts
I also tried using my company's internal code search and I cannot find environment variables using C_INCLUDE_DIRS.
I was going to hold this over for a bit longer. 2 weeks and if no one says anything then go ahead?
@MaskRay thanks for the info!
The main thing to worry about, clearly, is what happens as the change percolates downstream and into distros. But you probably know more about that than I do anyhow. :)
Yes, that is why I am a bit hesitant; as I know people are not always paying attention to mailing lists and maybe even notice something like this, even if in use. The kinds of changes
that even if they were sitting around for 6 month still come as a surprise.