Traffic annotation auditor updated to accept tool from arbitrary path.

Traffic annotation auditor would need to accept its clang tool from arbitrary
(non-default) path and is updated to accept a switch for this.

Bug: 656607
Bug: 690323
Change-Id: I071718f0410ecc636d71cbfc4fa9f18f9fcf1d19
Reviewed-on: https://chromium-review.googlesource.com/634908
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501592}

tools/clang/scripts/run_tool.py

@@ -262,14 +262,17 @@ def main():
   parser.add_argument(
       '--tool-args', nargs='*',
       help='optional arguments passed to the tool')
+  parser.add_argument(
+      '--tool-path', nargs='?',
+      help='optional path to the tool directory')
   args = parser.parse_args(argv)
 
-  os.environ['PATH'] = '%s%s%s' % (
-      os.path.abspath(os.path.join(
+  if args.tool_path:
+    tool_path = os.path.abspath(args.tool_path)
+  else:
+    tool_path = os.path.abspath(os.path.join(
           os.path.dirname(__file__),
-          '../../../third_party/llvm-build/Release+Asserts/bin')),
-      os.pathsep,
-      os.environ['PATH'])
+          '../../../third_party/llvm-build/Release+Asserts/bin'))
 
   if args.generate_compdb:
     with open(os.path.join(args.p, 'compile_commands.json'), 'w') as f:
@@ -298,7 +301,8 @@ def main():
     print 'Shard %d-of-%d will process %d entries out of %d' % (
         shard_number, shard_count, len(source_filenames), total_length)
 
-  dispatcher = _CompilerDispatcher(args.tool, args.tool_args,
+  dispatcher = _CompilerDispatcher(os.path.join(tool_path, args.tool),
+                                   args.tool_args,
                                    args.p,
                                    source_filenames)
   dispatcher.Run()

tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

@@ -4,6 +4,9 @@
 
 #include "tools/traffic_annotation/auditor/traffic_annotation_auditor.h"
 
+#include <stdio.h>
+
+#include "base/files/dir_reader_posix.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/process/launch.h"
@@ -11,6 +14,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
@@ -65,14 +69,27 @@ const std::string kBlockTypes[] = {"ASSIGNMENT", "ANNOTATION", "CALL"};
 
 const base::FilePath kSafeListPath(
     FILE_PATH_LITERAL("tools/traffic_annotation/auditor/safe_list.txt"));
+
+// The folder that includes the latest Clang built-in library. Inside this
+// folder, there should be another folder with version number, like
+// '.../lib/clang/6.0.0', which would be passed to the clang tool.
+const base::FilePath kClangLibraryPath(
+    FILE_PATH_LITERAL("third_party/llvm-build/Release+Asserts/lib/clang"));
+
 }  // namespace
 
 TrafficAnnotationAuditor::TrafficAnnotationAuditor(
     const base::FilePath& source_path,
-    const base::FilePath& build_path)
+    const base::FilePath& build_path,
+    const base::FilePath& clang_tool_path)
     : source_path_(source_path),
       build_path_(build_path),
-      safe_list_loaded_(false) {}
+      clang_tool_path_(clang_tool_path),
+      safe_list_loaded_(false) {
+  DCHECK(!source_path.empty());
+  DCHECK(!build_path.empty());
+  DCHECK(!clang_tool_path.empty());
+}
 
 TrafficAnnotationAuditor::~TrafficAnnotationAuditor() {}
 
@@ -83,11 +100,36 @@ int TrafficAnnotationAuditor::ComputeHashValue(const std::string& unique_id) {
                             : -1;
 }
 
+base::FilePath TrafficAnnotationAuditor::GetClangLibraryPath() {
+  base::FilePath base_path = source_path_.Append(kClangLibraryPath);
+  std::string library_folder;
+  base::DirReaderPosix dir_reader(base_path.MaybeAsASCII().c_str());
+
+  while (dir_reader.Next()) {
+    if (strcmp(dir_reader.name(), ".") && strcmp(dir_reader.name(), "..")) {
+      library_folder = dir_reader.name();
+      break;
+    }
+  }
+
+  if (library_folder.empty()) {
+    return base::FilePath();
+  } else {
+#if defined(OS_WIN)
+    return base_path.Append(base::ASCIIToUTF16(library_folder));
+#else
+    return base_path.Append(library_folder);
+#endif
+  }
+}
+
 bool TrafficAnnotationAuditor::RunClangTool(
     const std::vector<std::string>& path_filters,
     const bool full_run) {
   if (!safe_list_loaded_ && !LoadSafeList())
     return false;
+
+  // Create a file to pass options to clang scripts.
   base::FilePath options_filepath;
   if (!base::CreateTemporaryFile(&options_filepath)) {
     LOG(ERROR) << "Could not create temporary options file.";
@@ -98,9 +140,17 @@ bool TrafficAnnotationAuditor::RunClangTool(
     LOG(ERROR) << "Could not create temporary options file.";
     return false;
   }
-  fprintf(options_file,
-          "--generate-compdb --tool=traffic_annotation_extractor -p=%s ",
-          build_path_.MaybeAsASCII().c_str());
+
+  // As the checked out clang tool may be in a directory different from the
+  // default one (third_party/llvm-buid/Release+Asserts/bin), its path and
+  // clang's library folder should be passed to the run_tool.py script.
+  fprintf(
+      options_file,
+      "--generate-compdb --tool=traffic_annotation_extractor -p=%s "
+      "--tool-path=%s --tool-args=--extra-arg=-resource-dir=%s ",
+      build_path_.MaybeAsASCII().c_str(),
+      base::MakeAbsoluteFilePath(clang_tool_path_).MaybeAsASCII().c_str(),
+      base::MakeAbsoluteFilePath(GetClangLibraryPath()).MaybeAsASCII().c_str());
 
   // |safe_list_[ALL]| is not passed when |full_run| is happening as there is
   // no way to pass it to run_tools.py except enumerating all alternatives.
@@ -146,6 +196,7 @@ bool TrafficAnnotationAuditor::RunClangTool(
   cmdline.AppendArg(base::StringPrintf(
       "--options-file=%s", options_filepath.MaybeAsASCII().c_str()));
 
+  // Run, and clean after.
   bool result = base::GetAppOutput(cmdline, &clang_tool_raw_output_);
 
   base::DeleteFile(options_filepath, false);

tools/traffic_annotation/auditor/traffic_annotation_auditor.h

@@ -40,8 +40,13 @@ struct AuditorException {
 
 class TrafficAnnotationAuditor {
  public:
+  // Creates an auditor object, storing the following paths:
+  //   |source_path|: Path to the src directory.
+  //   |build_path|: Path to a compiled build directory.
+  //   |clang_tool_path|: Path to the 'traffic_annotation_extractor' clang tool.
   TrafficAnnotationAuditor(const base::FilePath& source_path,
-                           const base::FilePath& build_path);
+                           const base::FilePath& build_path,
+                           const base::FilePath& clang_tool_path);
   ~TrafficAnnotationAuditor();
 
   // Runs traffic_annotation_extractor clang tool and puts its output in
@@ -136,9 +141,13 @@ class TrafficAnnotationAuditor {
     gn_file_for_test_ = file_path;
   }
 
+  // Returns the path to clang internal libraries.
+  base::FilePath GetClangLibraryPath();
+
  private:
   const base::FilePath source_path_;
   const base::FilePath build_path_;
+  const base::FilePath clang_tool_path_;
 
   std::string clang_tool_raw_output_;
   std::vector<AnnotationInstance> extracted_annotations_;

tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc

@@ -32,6 +32,9 @@ Options:
   --source-path       Optional path to the src directory. If not provided and
                       build-path is available, assumed to be 'build-path/../..',
                       otherwise current directory.
+  --tool-path         Optional path to traffic_annotation_extractor clang tool.
+                      If not specified, it's assumed to be in the same path as
+                      auditor's executable.
   --extractor-output  Optional path to the temporary file that extracted
                       annotations will be stored into.
   --extracted-input   Optional path to the file that temporary extracted
@@ -284,6 +287,7 @@ int main(int argc, char* argv[]) {
 
   base::FilePath build_path = command_line.GetSwitchValuePath("build-path");
   base::FilePath source_path = command_line.GetSwitchValuePath("source-path");
+  base::FilePath tool_path = command_line.GetSwitchValuePath("tool-path");
   base::FilePath extractor_output =
       command_line.GetSwitchValuePath("extractor-output");
   base::FilePath extractor_input =
@@ -302,6 +306,11 @@ int main(int argc, char* argv[]) {
   path_filters = command_line.GetArgs();
 #endif
 
+  // If tool path is not specified, assume it is in the same path as this
+  // executable.
+  if (tool_path.empty())
+    tool_path = command_line.GetProgram().DirName();
+
   // If source path is not provided, guess it using build path or current
   // directory.
   if (source_path.empty()) {
@@ -312,7 +321,7 @@ int main(int argc, char* argv[]) {
                         .Append(base::FilePath::kParentDirectory);
   }
 
-  TrafficAnnotationAuditor auditor(source_path, build_path);
+  TrafficAnnotationAuditor auditor(source_path, build_path, tool_path);
 
   // Extract annotations.
   if (extractor_input.empty()) {
@@ -324,8 +333,10 @@ int main(int argc, char* argv[]) {
              "extracted annotations already exist.\n";
       return 1;
     }
-    if (!auditor.RunClangTool(path_filters, full_run))
+    if (!auditor.RunClangTool(path_filters, full_run)) {
+      LOG(ERROR) << "Failed to run clang tool.";
       return 1;
+    }
 
     // Write extractor output if requested.
     if (!extractor_output.empty()) {

tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "tools/traffic_annotation/auditor/traffic_annotation_auditor.h"
+
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
@@ -35,6 +36,12 @@ const char* kRelevantFiles[] = {
     "tools/traffic_annotation/auditor/tests/relevant_file_name_and_content.cc",
     "tools/traffic_annotation/auditor/tests/relevant_file_name_and_content.mm"};
 
+const base::FilePath kTestsFolder(
+    FILE_PATH_LITERAL("tools/traffic_annotation/auditor/tests"));
+
+const base::FilePath kClangToolPath(
+    FILE_PATH_LITERAL("tools/traffic_annotation/bin"));
+
 const base::FilePath kDownstreamUnittests(
     FILE_PATH_LITERAL("tools/traffic_annotation/scripts/"
                       "annotations_xml_downstream_caller.py"));
@@ -51,16 +58,29 @@ class TrafficAnnotationAuditorTest : public ::testing::Test {
       return;
     }
 
-    tests_folder_ = source_path_.Append(FILE_PATH_LITERAL("tools"))
-                        .Append(FILE_PATH_LITERAL("traffic_annotation"))
-                        .Append(FILE_PATH_LITERAL("auditor"))
-                        .Append(FILE_PATH_LITERAL("tests"));
-    auditor_ =
-        base::MakeUnique<TrafficAnnotationAuditor>(source_path(), build_path());
+    tests_folder_ = source_path_.Append(kTestsFolder);
+
+#if defined(OS_WIN)
+    base::FilePath platform_name(FILE_PATH_LITERAL("win32"));
+#elif defined(OS_LINUX)
+    base::FilePath platform_name(FILE_PATH_LITERAL("linux64"));
+#elif defined(OS_MACOSX)
+    base::FilePath platform_name(FILE_PATH_LITERAL("mac"));
+#else
+    NOTREACHED() << "Unexpected platform.";
+#endif
+
+    base::FilePath clang_tool_path =
+        source_path_.Append(kClangToolPath).Append(platform_name);
+
+    // As build path is not available and not used in tests, the default (empty)
+    // build path is passed to auditor.
+    auditor_ = std::make_unique<TrafficAnnotationAuditor>(
+        source_path_, source_path_.Append(FILE_PATH_LITERAL("out/Default")),
+        clang_tool_path);
   }
 
   const base::FilePath source_path() const { return source_path_; }
-  const base::FilePath build_path() const { return build_path_; }
   const base::FilePath tests_folder() const { return tests_folder_; };
   TrafficAnnotationAuditor& auditor() { return *auditor_; }
 
@@ -82,9 +102,6 @@ class TrafficAnnotationAuditorTest : public ::testing::Test {
 
  private:
   base::FilePath source_path_;
-  base::FilePath build_path_;  // Currently stays empty. Will be set if access
-                               // to a compiled build directory would be
-                               // granted.
   base::FilePath tests_folder_;
   std::unique_ptr<TrafficAnnotationAuditor> auditor_;
 };
@@ -850,4 +867,10 @@ TEST_F(TrafficAnnotationAuditorTest, AnnotationsDownstreamUnittests) {
   tests_result = system(cmdline.GetCommandLineString().c_str());
 #endif
   EXPECT_EQ(0, tests_result);
+}
+
+// Tests if AnnotationInstance::GetClangLibraryPath finds a path.
+TEST_F(TrafficAnnotationAuditorTest, GetClangLibraryPath) {
+  base::FilePath clang_library = auditor().GetClangLibraryPath();
+  EXPECT_FALSE(clang_library.empty());
 }
\ No newline at end of file