Consolidate Xcode Project Setup

tools/gn/xcode_writer.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/xcode_writer.h"
 
 #include <iomanip>
 #include <map>
 #include <memory>
 #include <sstream>
@@ skipping 23 matching lines @@
 using TargetToFileList = std::unordered_map<const Target*, Target::FileList>;
 using TargetToNativeTarget =
     std::unordered_map<const Target*, PBXNativeTarget*>;
 using FileToTargets = std::map<SourceFile,
                                std::vector<const Target*>,
                                bool (*)(const SourceFile&, const SourceFile&)>;
 
 const char kEarlGreyFileNameIdentifier[] = "egtest.mm";
 const char kXCTestFileNameIdentifier[] = "xctest.mm";
 const char kXCTestModuleTargetNamePostfix[] = "_module";
-const char kXCTestFileReferenceFolder[] = "xctests/";
 
 struct SafeEnvironmentVariableInfo {
   const char* name;
   bool capture_at_generation;
 };
 
 SafeEnvironmentVariableInfo kSafeEnvironmentVariables[] = {
     {"HOME", true}, {"LANG", true},    {"PATH", true},
     {"USER", true}, {"TMPDIR", false},
 };
 
-bool CompareSourceFiles(const SourceFile& lhs, const SourceFile& rhs) {
-  if (lhs.GetName() < rhs.GetName())
-    return true;
-  else if (lhs.GetName() > rhs.GetName())
-    return false;
-  else
-    return lhs.value() < rhs.value();
-}
-
 XcodeWriter::TargetOsType GetTargetOs(const Args& args) {
   const Value* target_os_value = args.GetArgOverride(variables::kTargetOs);
   if (target_os_value) {
     if (target_os_value->type() == Value::STRING) {
       if (target_os_value->string_value() == "ios")
         return XcodeWriter::WRITER_TARGET_OS_IOS;
     }
   }
   return XcodeWriter::WRITER_TARGET_OS_MACOS;
 }
@@ skipping 39 matching lines @@
 }
 
 bool IsXCTestModuleTarget(const Target* target) {
   return target->output_type() == Target::CREATE_BUNDLE &&
          target->bundle_data().product_type() ==
              "com.apple.product-type.bundle.unit-test" &&
          base::EndsWith(target->label().name(), kXCTestModuleTargetNamePostfix,
                         base::CompareCase::SENSITIVE);
 }
 
+bool IsXCTestFile(const SourceFile& file) {
+  return base::EndsWith(file.GetName(), kEarlGreyFileNameIdentifier,
+                        base::CompareCase::SENSITIVE) ||
+         base::EndsWith(file.GetName(), kXCTestFileNameIdentifier,
+                        base::CompareCase::SENSITIVE);
+}
+
 const Target* FindXCTestApplicationTarget(
     const Target* xctest_module_target,
     const std::vector<const Target*>& targets) {
   DCHECK(IsXCTestModuleTarget(xctest_module_target));
   DCHECK(base::EndsWith(xctest_module_target->label().name(),
                         kXCTestModuleTargetNamePostfix,
                         base::CompareCase::SENSITIVE));
   std::string application_target_name =
       xctest_module_target->label().name().substr(
           0, xctest_module_target->label().name().size() -
@@ skipping 20 matching lines @@
 
 // Searches the list of xctest files recursively under |target|.
 void SearchXCTestFiles(const Target* target,
                        TargetToFileList* xctest_files_per_target) {
   // Early return if already visited and processed.
   if (xctest_files_per_target->find(target) != xctest_files_per_target->end())
     return;
 
   Target::FileList xctest_files;
   for (const SourceFile& file : target->sources()) {
-    if (base::EndsWith(file.GetName(), kEarlGreyFileNameIdentifier,
-                       base::CompareCase::SENSITIVE) ||
-        base::EndsWith(file.GetName(), kXCTestFileNameIdentifier,
-                       base::CompareCase::SENSITIVE)) {
+    if (IsXCTestFile(file)) {
       xctest_files.push_back(file);
     }
   }
 
   // Call recursively on public and private deps.
   for (const auto& target : target->public_deps()) {
     SearchXCTestFiles(target.ptr, xctest_files_per_target);
     const Target::FileList& deps_xctest_files =
         (*xctest_files_per_target)[target.ptr];
     xctest_files.insert(xctest_files.end(), deps_xctest_files.begin(),
@@ skipping 23 matching lines @@
     std::vector<Target::FileList>* file_lists) {
   TargetToFileList xctest_files_per_target;
 
   for (const Target* target : application_targets) {
     DCHECK(IsApplicationTarget(target));
     SearchXCTestFiles(target, &xctest_files_per_target);
     file_lists->push_back(xctest_files_per_target[target]);
   }
 }
 
-// Maps each xctest file to a list of xctest application targets that contains
-// the file.
-void MapXCTestFileToApplicationTargets(
-    const std::vector<const Target*>& xctest_application_targets,
-    const std::vector<Target::FileList>& xctest_file_lists,
-    FileToTargets* xctest_file_to_application_targets) {
-  DCHECK_EQ(xctest_application_targets.size(), xctest_file_lists.size());
-
-  for (size_t i = 0; i < xctest_application_targets.size(); ++i) {
-    const Target* xctest_application_target = xctest_application_targets[i];
-    DCHECK(IsApplicationTarget(xctest_application_target));
-
-    for (const SourceFile& source : xctest_file_lists[i]) {
-      auto iter = xctest_file_to_application_targets->find(source);
-      if (iter == xctest_file_to_application_targets->end()) {
-        iter =
-            xctest_file_to_application_targets
-                ->insert(std::make_pair(source, std::vector<const Target*>()))
-                .first;
-      }
-      iter->second.push_back(xctest_application_target);
-    }
-  }
-}
-
 class CollectPBXObjectsPerClassHelper : public PBXObjectVisitor {
  public:
   CollectPBXObjectsPerClassHelper() {}
 
   void Visit(PBXObject* object) override {
     DCHECK(object);
     objects_per_class_[object->Class()].push_back(object);
   }
 
   const std::map<PBXObjectClass, std::vector<const PBXObject*>>&
@@ skipping 88 matching lines @@
     config_name = config_name.substr(0, separator);
 
   std::vector<const Target*> targets;
   std::vector<const Target*> all_targets = builder.GetAllResolvedTargets();
   if (!XcodeWriter::FilterTargets(build_settings, all_targets,
                                   dir_filters_string, &targets, err)) {
     return false;
   }
 
   XcodeWriter workspace(workspace_name);
-  workspace.CreateProductsProject(targets, attributes, source_path, config_name,
-                                  root_target_name, ninja_extra_args,
-                                  build_settings, target_os);
-
-  workspace.CreateSourcesProject(
-      all_targets, build_settings->build_dir(), attributes, source_path,
-      build_settings->root_path_utf8(), config_name, target_os);
+  workspace.CreateProductsProject(targets, all_targets, attributes, source_path,
+                                  config_name, root_target_name,
+                                  ninja_extra_args, build_settings, target_os);
 
   return workspace.WriteFiles(build_settings, err);
 }
 
 XcodeWriter::XcodeWriter(const std::string& name) : name_(name) {
   if (name_.empty())
     name_.assign("all");
 }
 
 XcodeWriter::~XcodeWriter() {}
@@ skipping 59 matching lines @@
   for (const Target* target : targets) {
     if (!IsXCTestModuleTarget(target))
       continue;
 
     xctest_module_targets->push_back(target);
   }
 }
 
 void XcodeWriter::CreateProductsProject(
     const std::vector<const Target*>& targets,
+    const std::vector<const Target*>& all_targets,
     const PBXAttributes& attributes,
     const std::string& source_path,
     const std::string& config_name,
     const std::string& root_target,
     const std::string& ninja_extra_args,
     const BuildSettings* build_settings,
     TargetOsType target_os) {
   std::unique_ptr<PBXProject> main_project(
       new PBXProject("products", config_name, source_path, attributes));
+  SourceDir source_dir("//");
+
+  // Add all source files for indexing.
+  std::vector<SourceFile> sources;
+  for (const Target* target : all_targets) {
+    for (const SourceFile& source : target->sources()) {
+      if (IsStringInOutputDir(build_settings->build_dir(), source.value()))
+        continue;
+
+      sources.push_back(source);
+    }
+  }
+
+  // Sort sources to ensure determinisn of the project file generation and
+  // remove duplicate reference to the source files (can happen due to the
+  // bundle_data targets).
+  std::sort(sources.begin(), sources.end());
+  sources.erase(std::unique(sources.begin(), sources.end()), sources.end());
+
+  for (const SourceFile& source : sources) {
+    std::string source_file = RebasePath(source.value(), source_dir,
+                                         build_settings->root_path_utf8());
+    main_project->AddSourceFileToIndexingTarget(source_file, source_file,
+                                                CompilerFlags::NONE);
+  }
 
   // Filter xctest module and application targets and find list of xctest files
   // recursively under them.
   std::vector<const Target*> xctest_module_targets;
   FilterXCTestModuleTargets(targets, &xctest_module_targets);
 
   std::vector<const Target*> xctest_application_targets;
   FindXCTestApplicationTargets(xctest_module_targets, targets,
                                &xctest_application_targets);
   DCHECK_EQ(xctest_module_targets.size(), xctest_application_targets.size());
@@ skipping 46 matching lines @@
                            .GetBundleRootDirOutput(target->settings())
                            .value(),
                        build_settings->build_dir()),
             target->bundle_data().product_type(),
             GetBuildScript(target->label().name(), ninja_extra_args, env.get()),
             extra_attributes);
 
         if (!IsXCTestModuleTarget(target))
           continue;
 
-        // Populate |xctest_application_to_module_native_target| for XCTest
-        // module targets.
-        const Target* application_target =
-            FindXCTestApplicationTarget(target, xctest_application_targets);
-        xctest_application_to_module_native_target.insert(
-            std::make_pair(application_target, native_target));
+        // Add xctest files to the "Compiler Sources" of corresponding xctest
+        // native targets.
+        const Target::FileList& xctest_file_list =
+            xctest_file_lists[std::find(xctest_module_targets.begin(),

[sdefresne, 2017/01/17 10:08:40]

nit: I find this line really confusing (I had to read it three time before
parsing it correctly), I suggest introducing a local variable to make it clearer

    auto iter = std::find(xctest_module_targets.begin(),
                          xctest_module_targets.end(), target);
    const size_t xctest_module_target_index = std::distance(
        xctest_module_targets.begin(), iter);
    const Target::FileList& xctest_file_lists[xctest_module_target_index];

Or maybe better, change xctest_file_lists to be a std::map<const Target*,
Target::FileList> so that you do not have to keep two std::vector<> indices in
sync.

[liaoyuke, 2017/01/17 18:35:19]

Done.

+                                        xctest_module_targets.end(), target) -
+                              xctest_module_targets.begin()];
+        for (const SourceFile& source : xctest_file_list) {
+          std::string source_path = RebasePath(
+              source.value(), source_dir, build_settings->root_path_utf8());
+
+          // Test files need to be known to Xcode for proper indexing and for
+          // discovery of tests function for XCTest, but the compilation is done
+          // via ninja and thus must prevent Xcode from compiling the files by
+          // adding '-help' as per file compiler flag.
+          main_project->AddSourceFile(source_path, source_path,
+                                      CompilerFlags::HELP, native_target);
+        }
 
         break;
       }
 
       default:
         break;
     }
   }
 
-  FileToTargets xctest_file_to_application_targets(CompareSourceFiles);
-  MapXCTestFileToApplicationTargets(xctest_application_targets,
-                                    xctest_file_lists,
-                                    &xctest_file_to_application_targets);
-
-  // Add xctest files to the "Compiler Sources" of corresponding xctest native
-  // targets.
-  SourceDir source_dir("//");
-  for (const auto& item : xctest_file_to_application_targets) {
-    const SourceFile& source = item.first;
-    for (const Target* xctest_application_target : item.second) {
-      std::string navigator_path =
-          kXCTestFileReferenceFolder + source.GetName();
-      std::string source_path = RebasePath(source.value(), source_dir,
-                                           build_settings->root_path_utf8());
-      PBXNativeTarget* xctest_module_native_target =
-          xctest_application_to_module_native_target[xctest_application_target];
-
-      // Test files need to be known to Xcode for proper indexing and for
-      // discovery of tests function for XCTest, but the compilation is done
-      // via ninja and thus must prevent Xcode from compiling the files by
-      // adding '-help' as per file compiler flag.
-      main_project->AddSourceFile(navigator_path, source_path,
-                                  CompilerFlags::HELP,
-                                  xctest_module_native_target);
-    }
-  }
-
   projects_.push_back(std::move(main_project));
 }
 
-void XcodeWriter::CreateSourcesProject(
-    const std::vector<const Target*>& targets,
-    const SourceDir& root_build_dir,
-    const PBXAttributes& attributes,
-    const std::string& source_path,
-    const std::string& absolute_source_path,
-    const std::string& config_name,
-    TargetOsType target_os) {
-  std::vector<SourceFile> sources;
-  for (const Target* target : targets) {
-    for (const SourceFile& source : target->sources()) {
-      if (IsStringInOutputDir(root_build_dir, source.value()))
-        continue;
-
-      sources.push_back(source);
-    }
-  }
-
-  std::unique_ptr<PBXProject> sources_for_indexing(
-      new PBXProject("sources", config_name, source_path, attributes));
-
-  // Sort sources to ensure determinisn of the project file generation and
-  // remove duplicate reference to the source files (can happen due to the
-  // bundle_data targets).
-  std::sort(sources.begin(), sources.end());
-  sources.erase(std::unique(sources.begin(), sources.end()), sources.end());
-
-  SourceDir source_dir("//");
-  for (const SourceFile& source : sources) {
-    std::string source_file =
-        RebasePath(source.value(), source_dir, absolute_source_path);
-    sources_for_indexing->AddSourceFileToIndexingTarget(
-        source_file, source_file, CompilerFlags::NONE);
-  }
-
-  projects_.push_back(std::move(sources_for_indexing));
-}
-
 bool XcodeWriter::WriteFiles(const BuildSettings* build_settings, Err* err) {
   for (const auto& project : projects_) {
     if (!WriteProjectFile(build_settings, project.get(), err))
       return false;
   }
 
   SourceFile xcworkspacedata_file =
       build_settings->build_dir().ResolveRelativeFile(
           Value(nullptr, name_ + ".xcworkspace/contents.xcworkspacedata"), err);
   if (xcworkspacedata_file.is_null())
@@ skipping 55 matching lines @@
     for (auto* object : pair.second) {
       object->Print(out, 2);
     }
     out << "/* End " << ToString(pair.first) << " section */\n";
   }
 
   out << "\t};\n"
       << "\trootObject = " << project->Reference() << ";\n"
       << "}\n";
 }