Allow directories for GN data lists. Allow the members of the "data" variable in GN targets to be either files or directories. This means storing them as strings rather than SourceFile objects, and rebasing as necessary. The documentation is updated accordingly. Added a move constructor for OutputFile. While move constructors are discouraged, OutputFile is just a wrapper around a std::string for type checking, and I think this is in the spirit of things. Review URL: https://codereview.chromium.org/1155303008 Cr-Original-Commit-Position: refs/heads/master@{#332712} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: e903c0f86b126b25fedaa0439230da1576aeb476
diff --git a/tools/gn/command_refs.cc b/tools/gn/command_refs.cc index 57ea824..9c89114 100644 --- a/tools/gn/command_refs.cc +++ b/tools/gn/command_refs.cc
@@ -138,7 +138,7 @@ return true; } for (const auto& cur_file : target->data()) { - if (cur_file == file) + if (cur_file == file.value()) return true; } return false;
diff --git a/tools/gn/output_file.cc b/tools/gn/output_file.cc index 1b6db62..12845fb 100644 --- a/tools/gn/output_file.cc +++ b/tools/gn/output_file.cc
@@ -10,8 +10,12 @@ OutputFile::OutputFile() : value_() { } -OutputFile::OutputFile(const base::StringPiece& str) - : value_(str.data(), str.size()) { +OutputFile::OutputFile(std::string&& v) + : value_(v) { +} + +OutputFile::OutputFile(const std::string& v) + : value_(v) { } OutputFile::OutputFile(const BuildSettings* build_settings,
diff --git a/tools/gn/output_file.h b/tools/gn/output_file.h index 3e0e99d..5f43a64 100644 --- a/tools/gn/output_file.h +++ b/tools/gn/output_file.h
@@ -17,7 +17,8 @@ class OutputFile { public: OutputFile(); - explicit OutputFile(const base::StringPiece& str); + explicit OutputFile(std::string&& v); + explicit OutputFile(const std::string& v); OutputFile(const BuildSettings* build_settings, const SourceFile& source_file); ~OutputFile();
diff --git a/tools/gn/runtime_deps.cc b/tools/gn/runtime_deps.cc index a8dfb0b..097044f 100644 --- a/tools/gn/runtime_deps.cc +++ b/tools/gn/runtime_deps.cc
@@ -36,13 +36,16 @@ deps->push_back(std::make_pair(output_file, source)); } -// Automatically converts a SourceFile to an OutputFile. -void AddIfNew(const SourceFile& source_file, +// Automatically converts a string that looks like a source to an OutputFile. +void AddIfNew(const std::string& str, const Target* source, RuntimeDepsVector* deps, std::set<OutputFile>* found_file) { - AddIfNew(OutputFile(source->settings()->build_settings(), source_file), - source, deps, found_file); + OutputFile output_file(RebasePath( + str, + source->settings()->build_settings()->build_dir(), + source->settings()->build_settings()->root_path_utf8())); + AddIfNew(output_file, source, deps, found_file); } // Returns the output file that the runtime deps considers for the given @@ -82,7 +85,7 @@ std::vector<SourceFile> outputs; target->action_values().GetOutputsAsSourceFiles(target, &outputs); for (const auto& output_file : outputs) - AddIfNew(output_file, target, deps, found_files); + AddIfNew(output_file.value(), target, deps, found_files); } // Non-data dependencies (both public and private). @@ -132,9 +135,9 @@ " (see \"gn help --runtime-deps-list-file\").\n" "\n" " To a first approximation, the runtime dependencies of a target are\n" - " the set of \"data\" files and the shared libraries from all transitive\n" - " dependencies. Executables and shared libraries are considered runtime\n" - " dependencies of themselves.\n" + " the set of \"data\" files, data directories, and the shared libraries\n" + " from all transitive dependencies. Executables and shared libraries are\n" + " considered runtime dependencies of themselves.\n" "\n" "Details\n" "\n"
diff --git a/tools/gn/runtime_deps_unittest.cc b/tools/gn/runtime_deps_unittest.cc index 4b73abc..0210b37 100644 --- a/tools/gn/runtime_deps_unittest.cc +++ b/tools/gn/runtime_deps_unittest.cc
@@ -49,17 +49,17 @@ Target stat(setup.settings(), Label(SourceDir("//"), "stat")); InitTargetWithType(setup, &stat, Target::STATIC_LIBRARY); - stat.data().push_back(SourceFile("//stat.dat")); + stat.data().push_back("//stat.dat"); ASSERT_TRUE(stat.OnResolved(&err)); Target shared(setup.settings(), Label(SourceDir("//"), "shared")); InitTargetWithType(setup, &shared, Target::SHARED_LIBRARY); - shared.data().push_back(SourceFile("//shared.dat")); + shared.data().push_back("//shared.dat"); ASSERT_TRUE(shared.OnResolved(&err)); Target set(setup.settings(), Label(SourceDir("//"), "set")); InitTargetWithType(setup, &set, Target::SOURCE_SET); - set.data().push_back(SourceFile("//set.dat")); + set.data().push_back("//set.dat"); ASSERT_TRUE(set.OnResolved(&err)); Target main(setup.settings(), Label(SourceDir("//"), "main")); @@ -67,7 +67,7 @@ main.private_deps().push_back(LabelTargetPair(&stat)); main.private_deps().push_back(LabelTargetPair(&shared)); main.private_deps().push_back(LabelTargetPair(&set)); - main.data().push_back(SourceFile("//main.dat")); + main.data().push_back("//main.dat"); ASSERT_TRUE(main.OnResolved(&err)); std::vector<std::pair<OutputFile, const Target*>> result = @@ -112,7 +112,7 @@ Target final_in(setup.settings(), Label(SourceDir("//"), "final_in")); InitTargetWithType(setup, &final_in, Target::SOURCE_SET); - final_in.data().push_back(SourceFile("//final_in.dat")); + final_in.data().push_back("//final_in.dat"); ASSERT_TRUE(final_in.OnResolved(&err)); Target datadep(setup.settings(), Label(SourceDir("//"), "datadep")); @@ -122,7 +122,7 @@ Target final_out(setup.settings(), Label(SourceDir("//"), "final_out")); InitTargetWithType(setup, &final_out, Target::SOURCE_SET); - final_out.data().push_back(SourceFile("//final_out.dat")); + final_out.data().push_back("//final_out.dat"); ASSERT_TRUE(final_out.OnResolved(&err)); Target dep(setup.settings(), Label(SourceDir("//"), "dep")); @@ -168,7 +168,7 @@ Target datadep(setup.settings(), Label(SourceDir("//"), "datadep")); InitTargetWithType(setup, &datadep, Target::ACTION); - datadep.data().push_back(SourceFile("//datadep.data")); + datadep.data().push_back("//datadep.data"); datadep.action_values().outputs() = SubstitutionList::MakeForTest("//datadep.output"); ASSERT_TRUE(datadep.OnResolved(&err)); @@ -176,14 +176,14 @@ Target datadep_copy(setup.settings(), Label(SourceDir("//"), "datadep_copy")); InitTargetWithType(setup, &datadep_copy, Target::COPY_FILES); datadep_copy.sources().push_back(SourceFile("//input")); - datadep_copy.data().push_back(SourceFile("//datadep_copy.data")); + datadep_copy.data().push_back("//datadep_copy.data"); datadep_copy.action_values().outputs() = SubstitutionList::MakeForTest("//datadep_copy.output"); ASSERT_TRUE(datadep_copy.OnResolved(&err)); Target dep(setup.settings(), Label(SourceDir("//"), "dep")); InitTargetWithType(setup, &dep, Target::ACTION); - dep.data().push_back(SourceFile("//dep.data")); + dep.data().push_back("//dep.data"); dep.action_values().outputs() = SubstitutionList::MakeForTest("//dep.output"); ASSERT_TRUE(dep.OnResolved(&err)); @@ -191,7 +191,7 @@ Target dep_copy(setup.settings(), Label(SourceDir("//"), "dep_copy")); InitTargetWithType(setup, &dep_copy, Target::COPY_FILES); dep_copy.sources().push_back(SourceFile("//input")); - dep_copy.data().push_back(SourceFile("//dep_copy.data")); + dep_copy.data().push_back("//dep_copy/data/"); // Tests a directory. dep_copy.action_values().outputs() = SubstitutionList::MakeForTest("//dep_copy.output"); ASSERT_TRUE(dep_copy.OnResolved(&err)); @@ -231,7 +231,7 @@ MakePair("../../dep.data", &dep)) != result.end()) << GetVectorDescription(result); EXPECT_TRUE(std::find(result.begin(), result.end(), - MakePair("../../dep_copy.data", &dep_copy)) != + MakePair("../../dep_copy/data/", &dep_copy)) != result.end()) << GetVectorDescription(result); // Explicitly asking for the runtime deps of an action target only includes
diff --git a/tools/gn/target.h b/tools/gn/target.h index 081f81e..0050048 100644 --- a/tools/gn/target.h +++ b/tools/gn/target.h
@@ -120,9 +120,11 @@ const FileList& inputs() const { return inputs_; } FileList& inputs() { return inputs_; } - // Runtime dependencies. - const FileList& data() const { return data_; } - FileList& data() { return data_; } + // Runtime dependencies. These are "file-like things" that can either be + // directories or files. They do not need to exist, these are just passed as + // runtime dependencies to external test systems as necessary. + const std::vector<std::string>& data() const { return data_; } + std::vector<std::string>& data() { return data_; } // Returns true if targets depending on this one should have an order // dependency. @@ -270,7 +272,7 @@ bool complete_static_lib_; bool testonly_; FileList inputs_; - FileList data_; + std::vector<std::string> data_; LabelTargetVector private_deps_; LabelTargetVector public_deps_;
diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc index f59a21b..6018147 100644 --- a/tools/gn/target_generator.cc +++ b/tools/gn/target_generator.cc
@@ -201,12 +201,40 @@ const Value* value = scope_->GetValue(variables::kData, true); if (!value) return true; - - Target::FileList dest_data; - if (!ExtractListOfRelativeFiles(scope_->settings()->build_settings(), *value, - scope_->GetSourceDir(), &dest_data, err_)) + if (!value->VerifyTypeIs(Value::LIST, err_)) return false; - target_->data().swap(dest_data); + + const std::vector<Value>& input_list = value->list_value(); + std::vector<std::string>& output_list = target_->data(); + output_list.reserve(input_list.size()); + + const SourceDir& dir = scope_->GetSourceDir(); + const std::string& root_path = + scope_->settings()->build_settings()->root_path_utf8(); + + for (size_t i = 0; i < input_list.size(); i++) { + const Value& input = input_list[i]; + if (!input.VerifyTypeIs(Value::STRING, err_)) + return false; + const std::string& input_str = input.string_value(); + + // Treat each input as either a file or a directory, depending on the + // last character. + if (!input_str.empty() && input_str[input_str.size() - 1] == '/') { + // Resolve as directory. + SourceDir resolved = + dir.ResolveRelativeDir(input, input_str, err_, root_path); + if (err_->has_error()) + return false; + output_list.push_back(resolved.value()); + } else { + // Resolve as file. + SourceFile resolved = dir.ResolveRelativeFile(input, err_, root_path); + if (err_->has_error()) + return false; + output_list.push_back(resolved.value()); + } + } return true; }
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc index 1cbfc9c..51a68d5 100644 --- a/tools/gn/variables.cc +++ b/tools/gn/variables.cc
@@ -509,11 +509,11 @@ const char kData_Help[] = "data: Runtime data file dependencies.\n" "\n" - " Lists files required to run the given target. These are typically\n" - " data files. The paths are interpreted as being relative to the current\n" - " build file. Since these are runtime dependencies, they do not affect\n" - " which targets are built or when. To declare input files to a script,\n" - " use \"inputs\".\n" + " Lists files or directories required to run the given target. These are\n" + " typically data files or directories of data files. The paths are\n" + " interpreted as being relative to the current build file. Since these\n" + " are runtime dependencies, they do not affect which targets are built\n" + " or when. To declare input files to a script, use \"inputs\".\n" "\n" " Appearing in the \"data\" section does not imply any special handling\n" " such as copying them to the output directory. This is just used for\n" @@ -526,6 +526,11 @@ " generated files both in the \"outputs\" list as well as the \"data\"\n" " list.\n" "\n" + " By convention, directories are be listed with a trailing slash:\n" + " data = [ \"test/data/\" ]\n" + " However, no verification is done on these so GN doesn't enforce this.\n" + " The paths are just rebased and passed along when requested.\n" + "\n" " See \"gn help runtime_deps\" for how these are used.\n"; const char kDataDeps[] = "data_deps";