Make the clang plugin look for path components.

Make the clang plugin look for path components.

Previously, we assumed that paths were relative to the base of the src
repository, which was bad. Now we'll search the entire path for the path
component "third_party/" instead.

BUG=carnitas
TEST=compiles and reduces clang spew on mac trybots w/o harming linux outputs.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76247

[Nico, 2011-02-24 16:00:04]

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
File tools/clang/plugins/ChromeClassTester.cpp (right):

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:35: // If the path to the file in
question has this as a path component, than
then

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:193: if (realpath(b.c_str(),
resolvedPath)) {
did you measure the build time impact of this?

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:200: // If we can find any of the
banned path components in this path, than
then

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:202: if (b.find(*it) !=
std::string::npos)
hm, i'm almost certain this will start banning directories it's not supposed to
at some point. Can you try to guess the source root at plugin startup somehow?
(e.g. realpath() on the input file, then look for "src" from the left and check
if chrome/chrome.gyp and build/common.gypi exist relative from that path)

[Elliot Glaysher, 2011-02-25 01:51:21]

This new version uses getcwd() and then strips off the cwd from realpath()ed
paths.

It's 10 seconds faster than the current plugin, so I assume that's noise.

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
File tools/clang/plugins/ChromeClassTester.cpp (right):

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:193: if (realpath(b.c_str(),
resolvedPath)) {
On 2011/02/24 16:00:05, Nico wrote:
> did you measure the build time impact of this?

On linux, this adds 14 seconds to make -j8 chrome (11m11 before, 11m25s after).

http://codereview.chromium.org/6577011/diff/1/tools/clang/plugins/ChromeClass...
tools/clang/plugins/ChromeClassTester.cpp:202: if (b.find(*it) !=
std::string::npos)
On 2011/02/24 16:00:05, Nico wrote:
> hm, i'm almost certain this will start banning directories it's not supposed
to
> at some point. Can you try to guess the source root at plugin startup somehow?
> (e.g. realpath() on the input file, then look for "src" from the left and
check
> if chrome/chrome.gyp and build/common.gypi exist relative from that path)

The relative paths will often be bellow the directories searched for; realpath()
will still be needed to resolve stuff.

[Nico, 2011-02-25 02:08:41]

I like this much better. Thanks for humoring me here.

http://codereview.chromium.org/6577011/diff/4001/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.cpp (right):

http://codereview.chromium.org/6577011/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:54: if (access(common.c_str(), F_OK)
!= -1) {
you can save quite a few stats here if you only check this if common ends with
"src"

http://codereview.chromium.org/6577011/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:65: llvm::errs() << "WARNING: Can't
figure out srcroot!\n";
emit an error?

http://codereview.chromium.org/6577011/diff/4001/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.h (right):

http://codereview.chromium.org/6577011/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:60: std::string srcroot_;
src_root_ for consistency with the method name