This is an archive of the discontinued LLVM Phabricator instance.

[cmake] Make gtest include directories a part of the library interface
ClosedPublic

Authored by labath on Aug 26 2020, 5:39 AM.

Details

Summary

This applies the same fix that D84748 did for macro definitions.
Appropriate include path is now automatically set for all libraries
which link against gtest targets, which avoids the need to set
include_directories in various parts of the project.

Diff Detail

Event Timeline

labath created this revision.Aug 26 2020, 5:39 AM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
labath requested review of this revision.Aug 26 2020, 5:39 AM
ldionne accepted this revision.Aug 26 2020, 7:09 AM

LGTM, but I'm not an owner for any of the projects touched by this change.

This revision is now accepted and ready to land.Aug 26 2020, 7:09 AM
teemperor accepted this revision.Aug 26 2020, 7:12 AM
teemperor added a subscriber: teemperor.

LGTM too

sivachandra accepted this revision.Aug 26 2020, 8:32 AM
sivachandra added a subscriber: sivachandra.

Libc change LGTM.

LGTM, but I'm not an owner for any of the projects touched by this change.

I picked you because you seemed interested in the overall direction that our cmake support is going :), and I believe that one of our biggest problem with cmake is the lack of a unified direction of where our cmake support is going. (Also, my previous go-to person for that (@beanz), seems to be busy with other stuff these days.)

LGTM, but I'm not an owner for any of the projects touched by this change.

I picked you because you seemed interested in the overall direction that our cmake support is going :), and I believe that one of our biggest problem with cmake is the lack of a unified direction of where our cmake support is going. (Also, my previous go-to person for that (@beanz), seems to be busy with other stuff these days.)

Yup, I am interested by that indeed :). I just wanted to make it clear my LGTM alone shouldn't be considered enough to commit to these other projects that might have active owners. But it does look like you've got enough approvals for an obviously correct improvement, IMO.