Commit bea9f37a authored by Mostyn Bramley-Moore's avatar Mostyn Bramley-Moore Committed by Commit Bot

Revert "drop unnecessary realpath usage in the FindBadConstructs plugin"

This reverts commit 07b2231e.

Reason for revert:

We need to be more conservative with plugin behaviour changes.
TODO: document the process, then attempt to re-land this change
via that process.

Original change's description:
> drop unnecessary realpath usage in the FindBadConstructs plugin
>
> All we need to do is ensure that there's a leading '/' in the path, we
> don't expect the path to exist necessarily, and we don't need to resolve
> symlinks.  We just want to be able to search for directories by doing a
> string search of the form "/foo/".  And since we don't use realpath
> anymore, this should work with distributed compilation systems like icecc.
>
> This will also avoid mis-classification of files in chromium checkouts
> that are inside a directory with the same name as one of the so-called
> "banned" directories, eg a checkout in /home/gen/chromium/.
>
> BUG=583454

Change-Id: Ic840387bb2244d967e66fd078684c56435fe940a
Reviewed-on: https://chromium-review.googlesource.com/986435Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Cr-Commit-Position: refs/heads/master@{#546928}
parent 5ab371bc
...@@ -95,17 +95,47 @@ ChromeClassTester::LocationType ChromeClassTester::ClassifyLocation( ...@@ -95,17 +95,47 @@ ChromeClassTester::LocationType ChromeClassTester::ClassifyLocation(
if (filename == "<scratch space>") if (filename == "<scratch space>")
return LocationType::kThirdParty; return LocationType::kThirdParty;
#if defined(LLVM_ON_UNIX)
// Resolve the symlinktastic relative path and make it absolute.
char resolvedPath[MAXPATHLEN];
if (options_.no_realpath) {
// Same reason as windows below.
filename.insert(filename.begin(), '/');
} else if (realpath(filename.c_str(), resolvedPath)) {
filename = resolvedPath;
}
#endif
#if defined(LLVM_ON_WIN32)
// Make path absolute.
if (options_.no_realpath) {
// This turns e.g. "gen/dir/file.cc" to "/gen/dir/file.cc" which lets the
// "/gen/" banned_dir work.
filename.insert(filename.begin(), '/');
} else {
// The Windows dance: Convert to UTF-16, call GetFullPathNameW, convert back
DWORD size_needed =
MultiByteToWideChar(CP_UTF8, 0, filename.data(), -1, nullptr, 0);
std::wstring utf16(size_needed, L'\0');
MultiByteToWideChar(CP_UTF8, 0, filename.data(), -1,
&utf16[0], size_needed);
size_needed = GetFullPathNameW(utf16.data(), 0, nullptr, nullptr);
std::wstring full_utf16(size_needed, L'\0');
GetFullPathNameW(utf16.data(), full_utf16.size(), &full_utf16[0], nullptr);
size_needed = WideCharToMultiByte(CP_UTF8, 0, full_utf16.data(), -1,
nullptr, 0, nullptr, nullptr);
filename.resize(size_needed);
WideCharToMultiByte(CP_UTF8, 0, full_utf16.data(), -1, &filename[0],
size_needed, nullptr, nullptr);
}
#endif
// When using distributed cross compilation build tools, file paths can have // When using distributed cross compilation build tools, file paths can have
// separators which differ from ones at this platform. Make them consistent. // separators which differ from ones at this platform. Make them consistent.
std::replace(filename.begin(), filename.end(), '\\', '/'); std::replace(filename.begin(), filename.end(), '\\', '/');
// Ensure that we can search for patterns of the form "/foo/" even
// if we have a relative path like "foo/bar.cc". We don't expect
// this transformed path to exist necessarily.
if (filename.front() != '/') {
filename.insert(0, 1, '/');
}
// Don't check autogenerated files. ninja puts them in $OUT_DIR/gen. // Don't check autogenerated files. ninja puts them in $OUT_DIR/gen.
if (filename.find("/gen/") != std::string::npos) if (filename.find("/gen/") != std::string::npos)
return LocationType::kThirdParty; return LocationType::kThirdParty;
......
...@@ -53,6 +53,8 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance, ...@@ -53,6 +53,8 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
// TODO(tsepez): Enable this by default once http://crbug.com/356815 // TODO(tsepez): Enable this by default once http://crbug.com/356815
// and http://crbug.com/356816 are fixed. // and http://crbug.com/356816 are fixed.
options_.check_enum_last_value = true; options_.check_enum_last_value = true;
} else if (args[i] == "no-realpath") {
options_.no_realpath = true;
} else if (args[i] == "check-ipc") { } else if (args[i] == "check-ipc") {
options_.check_ipc = true; options_.check_ipc = true;
} else { } else {
......
...@@ -11,6 +11,9 @@ struct Options { ...@@ -11,6 +11,9 @@ struct Options {
bool check_base_classes = false; bool check_base_classes = false;
bool enforce_in_thirdparty_webkit = false; // Use in Blink code itself bool enforce_in_thirdparty_webkit = false; // Use in Blink code itself
bool check_enum_last_value = false; bool check_enum_last_value = false;
// This is needed for some distributed build-sytems to respect banned
// paths. See https://crbug.com/583454 for details.
bool no_realpath = false;
bool check_ipc = false; bool check_ipc = false;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment