[metadata] Allow specification of rebase dir Change generated_file's rebase variable to be a source directory, specifying on which dir to rebase the final paths. Change-Id: Ic4d04427f66580d3e49f75d5079bedfb117b4d58 Reviewed-on: https://gn-review.googlesource.com/c/3580 Commit-Queue: Julie Hockett <juliehockett@google.com> Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/tools/gn/command_meta.cc b/tools/gn/command_meta.cc index f5f96b4..ecf07ef 100644 --- a/tools/gn/command_meta.cc +++ b/tools/gn/command_meta.cc
@@ -20,11 +20,36 @@ const char kMeta_HelpShort[] = "meta: List target metadata collection results."; const char kMeta_Help[] = R"(gn meta <out_dir> <target>* --data=<key>[,<key>*]* [--walk=<key>[,<key>*]*] - [--rebase] + [--rebase=<dest dir>] Lists collected metaresults of all given targets for the given data key(s), collecting metadata dependencies as specified by the given walk key(s). + See `gn help generated_file` for more information on the walk. + +Arguments + + <target(s)> + A list of target labels from which to initiate the walk. + + --data + A list of keys from which to extract data. In each target walked, its metadata + scope is checked for the presence of these keys. If present, the contents of + those variable in the scope are appended to the results list. + + --walk (optional) + A list of keys from which to control the walk. In each target walked, its + metadata scope is checked for the presence of any of these keys. If present, + the contents of those variables is checked to ensure that it is a label of + a valid dependency of the target and then added to the set of targets to walk. + If the empty string ("") is present in any of these keys, all deps and data_deps + are added to the walk set. + + --rebase (optional) + A destination directory onto which to rebase any paths found. If set, all + collected metadata will be rebased onto this path. This option will throw errors + if collected metadata is not a list of strings. + Examples gn meta out/Debug "//base/foo" --data=files @@ -39,17 +64,17 @@ Lists collected metaresults for the `files` key in the //base/foo:foo target and all of the dependencies listed in the `stop` key (and so on). - gn meta out/Debug "//base/foo" --data=files --rebase-files + gn meta out/Debug "//base/foo" --data=files --rebase="/" Lists collected metaresults for the `files` key in the //base/foo:foo target and all of its dependency tree, rebasing the strings in the `files` - key onto the source directory of the target's declaration. + key onto the source directory of the target's declaration relative to "/". )"; int RunMeta(const std::vector<std::string>& args) { if (args.size() == 0) { Err(Location(), "You're holding it wrong.", "Usage: \"gn meta <out_dir> <target>* --data=<key>[,<key>*] " - "[--walk=<key>[,<key>*]*] [--rebase-files]\"") + "[--walk=<key>[,<key>*]*] [--rebase=<dest dir>]\"") .PrintToStdout(); return 1; } @@ -59,7 +84,8 @@ return 1; const base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess(); - bool rebase_files = cmdline->HasSwitch(switches::kMetaRebaseFiles); + std::string rebase_dir = + cmdline->GetSwitchValueASCII(switches::kMetaRebaseFiles); std::string data_keys_str = cmdline->GetSwitchValueASCII(switches::kMetaDataKeys); std::string walk_keys_str = @@ -86,8 +112,9 @@ walk_keys_str, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); Err err; std::set<const Target*> targets_walked; - std::vector<Value> result = WalkMetadata(targets, data_keys, walk_keys, - rebase_files, &targets_walked, &err); + std::vector<Value> result = + WalkMetadata(targets, data_keys, walk_keys, SourceDir(rebase_dir), + &targets_walked, &err); if (err.has_error()) { err.PrintToStdout(); return 1;
diff --git a/tools/gn/desc_builder.cc b/tools/gn/desc_builder.cc index 3bc7e86..d4b4941 100644 --- a/tools/gn/desc_builder.cc +++ b/tools/gn/desc_builder.cc
@@ -476,8 +476,8 @@ res->SetKey(variables::kDataKeys, std::move(keys)); } if (what(variables::kRebase)) { - res->SetKey(variables::kRebase, - std::move(base::Value(target_->rebase()))); + res->SetWithoutPathExpansion(variables::kRebase, + RenderValue(target_->rebase())); } if (what(variables::kWalkKeys)) { base::ListValue keys;
diff --git a/tools/gn/generated_file_target_generator.cc b/tools/gn/generated_file_target_generator.cc index f953f38..1854d5b 100644 --- a/tools/gn/generated_file_target_generator.cc +++ b/tools/gn/generated_file_target_generator.cc
@@ -105,9 +105,19 @@ return true; if (!IsMetadataCollectionTarget(variables::kRebase, value->origin())) return false; - if (!value->VerifyTypeIs(Value::BOOLEAN, err_)) + if (!value->VerifyTypeIs(Value::STRING, err_)) return false; - target_->set_rebase(value->boolean_value()); + + if (value->string_value().empty()) + return true; // Treat empty string as the default and do nothing. + + const BuildSettings* build_settings = scope_->settings()->build_settings(); + SourceDir dir = scope_->GetSourceDir().ResolveRelativeDir( + *value, err_, build_settings->root_path_utf8()); + if (err_->has_error()) + return false; + + target_->set_rebase(dir); return true; }
diff --git a/tools/gn/metadata.cc b/tools/gn/metadata.cc index 605294b..3250b03 100644 --- a/tools/gn/metadata.cc +++ b/tools/gn/metadata.cc
@@ -4,10 +4,12 @@ #include "tools/gn/metadata.h" +#include "tools/gn/filesystem_utils.h" + bool Metadata::WalkStep(const BuildSettings* settings, const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::vector<Value>* next_walk_keys, std::vector<Value>* result, Err* err) const { @@ -24,22 +26,22 @@ continue; assert(iter->second.type() == Value::LIST); - if (rebase_files) { + if (!rebase_dir.is_null()) { for (const auto& val : iter->second.list_value()) { if (!val.VerifyTypeIs(Value::STRING, err)) return false; - // TODO(juliehockett): Do we want to consider absolute paths here? In - // which case we'd need to propagate the root_path_utf8 from - // build_settings as well. std::string filename = source_dir_.ResolveRelativeAs( /*as_file = */ true, val, err, settings->root_path_utf8()); if (err->has_error()) return false; - result->emplace_back(val.origin(), std::move(filename)); + + result->emplace_back( + val.origin(), + RebasePath(filename, rebase_dir, settings->root_path_utf8())); } } else { result->insert(result->end(), iter->second.list_value().begin(), - iter->second.list_value().end()); + iter->second.list_value().end()); } }
diff --git a/tools/gn/metadata.h b/tools/gn/metadata.h index 235542b..da79fdf 100644 --- a/tools/gn/metadata.h +++ b/tools/gn/metadata.h
@@ -53,7 +53,7 @@ bool WalkStep(const BuildSettings* settings, const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::vector<Value>* next_walk_keys, std::vector<Value>* result, Err* err) const;
diff --git a/tools/gn/metadata_unittest.cc b/tools/gn/metadata_unittest.cc index 2fcebfa..b159372 100644 --- a/tools/gn/metadata_unittest.cc +++ b/tools/gn/metadata_unittest.cc
@@ -58,8 +58,8 @@ Err err; EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, false, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir(), &next_walk_keys, + &results, &err)); EXPECT_FALSE(err.has_error()); EXPECT_EQ(next_walk_keys, expected_walk_keys); EXPECT_EQ(results, expected); @@ -84,16 +84,16 @@ std::vector<Value> results; std::vector<Value> expected; - expected.emplace_back(Value(nullptr, "/usr/home/files/foo.cpp")); - expected.emplace_back(Value(nullptr, "/usr/home/files/foo/bar.h")); + expected.emplace_back(Value(nullptr, "../home/files/foo.cpp")); + expected.emplace_back(Value(nullptr, "../home/files/foo/bar.h")); std::vector<Value> expected_walk_keys; expected_walk_keys.emplace_back(nullptr, ""); Err err; EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, true, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir("/usr/foo_dir/"), + &next_walk_keys, &results, &err)); EXPECT_FALSE(err.has_error()); EXPECT_EQ(next_walk_keys, expected_walk_keys); EXPECT_EQ(results, expected); @@ -119,8 +119,8 @@ Err err; EXPECT_FALSE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, true, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir("/foo_dir/"), + &next_walk_keys, &results, &err)); EXPECT_TRUE(err.has_error()); } @@ -146,8 +146,8 @@ Err err; EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, true, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir(), &next_walk_keys, + &results, &err)); EXPECT_FALSE(err.has_error()); EXPECT_EQ(next_walk_keys, expected_walk_keys); EXPECT_TRUE(results.empty()); @@ -168,8 +168,8 @@ Err err; EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, true, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir(), &next_walk_keys, + &results, &err)); EXPECT_FALSE(err.has_error()); EXPECT_EQ(next_walk_keys, expected_walk_keys); EXPECT_TRUE(results.empty()); @@ -196,8 +196,8 @@ Err err; EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys, - walk_keys, true, &next_walk_keys, &results, - &err)); + walk_keys, SourceDir(), &next_walk_keys, + &results, &err)); EXPECT_FALSE(err.has_error()); EXPECT_EQ(next_walk_keys, expected_walk_keys); EXPECT_TRUE(results.empty());
diff --git a/tools/gn/metadata_walk.cc b/tools/gn/metadata_walk.cc index 6ce9b88..dabe285 100644 --- a/tools/gn/metadata_walk.cc +++ b/tools/gn/metadata_walk.cc
@@ -8,14 +8,14 @@ const UniqueVector<const Target*>& targets_to_walk, const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::set<const Target*>* targets_walked, Err* err) { std::vector<Value> result; for (const auto* target : targets_to_walk) { auto pair = targets_walked->insert(target); if (pair.second) { - if (!target->GetMetadata(keys_to_extract, keys_to_walk, rebase_files, + if (!target->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir, &result, targets_walked, err)) return std::vector<Value>(); }
diff --git a/tools/gn/metadata_walk.h b/tools/gn/metadata_walk.h index 4601a3f..9f4f34d 100644 --- a/tools/gn/metadata_walk.h +++ b/tools/gn/metadata_walk.h
@@ -19,7 +19,7 @@ const UniqueVector<const Target*>& targets_to_walk, const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::set<const Target*>* targets_walked, Err* err);
diff --git a/tools/gn/metadata_walk_unittest.cc b/tools/gn/metadata_walk_unittest.cc index 4855131..4ceea7b 100644 --- a/tools/gn/metadata_walk_unittest.cc +++ b/tools/gn/metadata_walk_unittest.cc
@@ -51,8 +51,8 @@ Err err; std::set<const Target*> targets_walked; - std::vector<Value> result = - WalkMetadata(targets, data_keys, walk_keys, false, &targets_walked, &err); + std::vector<Value> result = WalkMetadata(targets, data_keys, walk_keys, + SourceDir(), &targets_walked, &err); EXPECT_FALSE(err.has_error()); std::vector<Value> expected; @@ -101,8 +101,8 @@ Err err; std::set<const Target*> targets_walked; - std::vector<Value> result = - WalkMetadata(targets, data_keys, walk_keys, false, &targets_walked, &err); + std::vector<Value> result = WalkMetadata(targets, data_keys, walk_keys, + SourceDir(), &targets_walked, &err); EXPECT_FALSE(err.has_error()); std::vector<Value> expected; @@ -158,8 +158,8 @@ Err err; std::set<const Target*> targets_walked; - std::vector<Value> result = - WalkMetadata(targets, data_keys, walk_keys, false, &targets_walked, &err); + std::vector<Value> result = WalkMetadata(targets, data_keys, walk_keys, + SourceDir(), &targets_walked, &err); EXPECT_FALSE(err.has_error()) << err.message(); std::vector<Value> expected; @@ -198,8 +198,8 @@ Err err; std::set<const Target*> targets_walked; - std::vector<Value> result = - WalkMetadata(targets, data_keys, walk_keys, false, &targets_walked, &err); + std::vector<Value> result = WalkMetadata(targets, data_keys, walk_keys, + SourceDir(), &targets_walked, &err); EXPECT_TRUE(result.empty()); EXPECT_TRUE(err.has_error()); EXPECT_EQ(err.message(),
diff --git a/tools/gn/target.cc b/tools/gn/target.cc index 89810c6..c5eebd3 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc
@@ -287,8 +287,7 @@ check_includes_(true), complete_static_lib_(false), testonly_(false), - toolchain_(nullptr), - rebase_(false) {} + toolchain_(nullptr) {} Target::~Target() = default; @@ -880,13 +879,13 @@ bool Target::GetMetadata(const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::vector<Value>* result, std::set<const Target*>* targets_walked, Err* err) const { std::vector<Value> next_walk_keys; if (!metadata_.WalkStep(settings()->build_settings(), keys_to_extract, - keys_to_walk, rebase_files, &next_walk_keys, result, + keys_to_walk, rebase_dir, &next_walk_keys, result, err)) return false; @@ -905,7 +904,7 @@ // If we haven't walked this dep yet, go down into it. auto pair = targets_walked->insert(dep.ptr); if (pair.second) { - if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_files, + if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir, result, targets_walked, err)) return false; } @@ -924,7 +923,7 @@ // If we haven't walked this dep yet, go down into it. auto pair = targets_walked->insert(dep.ptr); if (pair.second) { - if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_files, + if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir, result, targets_walked, err)) return false; }
diff --git a/tools/gn/target.h b/tools/gn/target.h index e75b97a..0087e9e 100644 --- a/tools/gn/target.h +++ b/tools/gn/target.h
@@ -154,7 +154,7 @@ // be called after the target is resolved. bool GetMetadata(const std::vector<std::string>& keys_to_extract, const std::vector<std::string>& keys_to_walk, - bool rebase_files, + const SourceDir& rebase_dir, std::vector<Value>* result, std::set<const Target*>* targets_walked, Err* err) const; @@ -168,8 +168,8 @@ void set_output_conversion(const Value& value) { output_conversion_ = value; } // Metadata collection methods for WriteData targets. - bool rebase() const { return rebase_; } - void set_rebase(bool value) { rebase_ = value; } + const SourceDir& rebase() const { return rebase_; } + void set_rebase(const SourceDir& value) { rebase_ = value; } const std::vector<std::string>& data_keys() const { return data_keys_; } std::vector<std::string>& data_keys() { return data_keys_; } const std::vector<std::string>& walk_keys() const { return walk_keys_; } @@ -426,7 +426,7 @@ Value contents_; // Value::NONE if metadata collection should occur. // GeneratedFile as metadata collection values. - bool rebase_; + SourceDir rebase_; std::vector<std::string> data_keys_; std::vector<std::string> walk_keys_;
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc index b4fa5f9..55605c6 100644 --- a/tools/gn/target_unittest.cc +++ b/tools/gn/target_unittest.cc
@@ -1118,7 +1118,7 @@ Err err; std::vector<Value> result; std::set<const Target*> targets; - one.GetMetadata(data_keys, walk_keys, false, &result, &targets, &err); + one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err); EXPECT_FALSE(err.has_error()); std::vector<Value> expected; @@ -1158,7 +1158,7 @@ Err err; std::vector<Value> result; std::set<const Target*> targets; - one.GetMetadata(data_keys, walk_keys, false, &result, &targets, &err); + one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err); EXPECT_FALSE(err.has_error()); std::vector<Value> expected; @@ -1207,7 +1207,7 @@ Err err; std::vector<Value> result; std::set<const Target*> targets; - one.GetMetadata(data_keys, walk_keys, false, &result, &targets, &err); + one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err); EXPECT_FALSE(err.has_error()) << err.message(); std::vector<Value> expected; @@ -1239,7 +1239,7 @@ Err err; std::vector<Value> result; std::set<const Target*> targets; - one.GetMetadata(data_keys, walk_keys, false, &result, &targets, &err); + one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err); EXPECT_TRUE(err.has_error()); EXPECT_EQ(err.message(), "I was expecting //foo:missing to be a dependency of "