Use WithoutPathExpansion on DictionaryValue in GN.
The normal 'Set' methods on DictionaryValue scan for '.' in the path and
assume that the '.' indexes into an inner DictionaryValue. At no point
does gn appear to desire this behavior, so call the WithoutPathExpansion
variants instead.
This is mandatory when the path is user supplied, as it may have a '.'
This change was motivated by a target named like 'x.y' which caused
gn desc config //:x.y --format=json
to produce something like
"//:x": {
"y": {
"args": [ ... ]
...
}
}
and similar issues when producing the project.json from the json ide
generator.
When the path is a constant which is known not to contain '.' there is
no behavioral change, but the extra work of scanning for the '.' is
avoided. In addition, if all uses are of the WithoutPathExpansion
variant then it becomes much less likely that similar errors will be
introduced in the future.
Review-Url: https://codereview.chromium.org/2363193002
Cr-Original-Commit-Position: refs/heads/master@{#421005}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: d51d3527da33b8f736dda1e99bcd9dc1586b366f
diff --git a/tools/gn/analyzer.cc b/tools/gn/analyzer.cc
index 011b77b..06f9336 100644
--- a/tools/gn/analyzer.cc
+++ b/tools/gn/analyzer.cc
@@ -102,7 +102,7 @@
void WriteString(base::DictionaryValue& dict,
const std::string& key,
const std::string& value) {
- dict.SetString(key, value);
+ dict.SetStringWithoutPathExpansion(key, value);
};
void WriteLabels(const Label& default_toolchain,
@@ -115,7 +115,7 @@
strings.push_back(l.GetUserVisibleName(default_toolchain));
std::sort(strings.begin(), strings.end());
value->AppendStrings(strings);
- dict.Set(key, std::move(value));
+ dict.SetWithoutPathExpansion(key, std::move(value));
}
Label AbsoluteOrSourceAbsoluteStringToLabel(const Label& default_toolchain,
@@ -207,7 +207,8 @@
if (outputs.compile_includes_all) {
auto compile_targets = base::WrapUnique(new base::ListValue());
compile_targets->AppendString("all");
- value->Set("compile_targets", std::move(compile_targets));
+ value->SetWithoutPathExpansion("compile_targets",
+ std::move(compile_targets));
} else {
WriteLabels(default_toolchain, *value, "compile_targets",
outputs.compile_labels);
diff --git a/tools/gn/command_desc.cc b/tools/gn/command_desc.cc
index acb8077..27117c5 100644
--- a/tools/gn/command_desc.cc
+++ b/tools/gn/command_desc.cc
@@ -462,16 +462,18 @@
auto res = base::MakeUnique<base::DictionaryValue>();
if (!target_matches.empty()) {
for (const auto* target : target_matches) {
- res->Set(target->label().GetUserVisibleName(
- target->settings()->default_toolchain_label()),
- DescBuilder::DescriptionForTarget(
- target, what_to_print, cmdline->HasSwitch(kAll),
- cmdline->HasSwitch(kTree), cmdline->HasSwitch(kBlame)));
+ res->SetWithoutPathExpansion(
+ target->label().GetUserVisibleName(
+ target->settings()->default_toolchain_label()),
+ DescBuilder::DescriptionForTarget(
+ target, what_to_print, cmdline->HasSwitch(kAll),
+ cmdline->HasSwitch(kTree), cmdline->HasSwitch(kBlame)));
}
} else if (!config_matches.empty()) {
for (const auto* config : config_matches) {
- res->Set(config->label().GetUserVisibleName(false),
- DescBuilder::DescriptionForConfig(config, what_to_print));
+ res->SetWithoutPathExpansion(
+ config->label().GetUserVisibleName(false),
+ DescBuilder::DescriptionForConfig(config, what_to_print));
}
}
std::string s;
diff --git a/tools/gn/desc_builder.cc b/tools/gn/desc_builder.cc
index aca148c..3e2aaa0 100644
--- a/tools/gn/desc_builder.cc
+++ b/tools/gn/desc_builder.cc
@@ -159,13 +159,14 @@
const ConfigValues& values) {
if (what(variables::kPrecompiledHeader) &&
!values.precompiled_header().empty()) {
- out->Set(variables::kPrecompiledHeader,
- RenderValue(values.precompiled_header(), true));
+ out->SetWithoutPathExpansion(
+ variables::kPrecompiledHeader,
+ RenderValue(values.precompiled_header(), true));
}
if (what(variables::kPrecompiledSource) &&
!values.precompiled_source().is_null()) {
- out->Set(variables::kPrecompiledSource,
- RenderValue(values.precompiled_source()));
+ out->SetWithoutPathExpansion(variables::kPrecompiledSource,
+ RenderValue(values.precompiled_source()));
}
}
@@ -185,14 +186,14 @@
const ConfigValues& values = config_->resolved_values();
if (what_.empty())
- res->SetString(
+ res->SetStringWithoutPathExpansion(
"toolchain",
config_->label().GetToolchainLabel().GetUserVisibleName(false));
if (what(variables::kConfigs) && !config_->configs().empty()) {
auto configs = base::MakeUnique<base::ListValue>();
FillInConfigVector(configs.get(), config_->configs().vector());
- res->Set(variables::kConfigs, std::move(configs));
+ res->SetWithoutPathExpansion(variables::kConfigs, std::move(configs));
}
#define CONFIG_VALUE_ARRAY_HANDLER(name, type) \
@@ -200,7 +201,7 @@
ValuePtr ptr = \
render_config_value_array<type>(values, &ConfigValues::name); \
if (ptr) { \
- res->Set(#name, std::move(ptr)); \
+ res->SetWithoutPathExpansion(#name, std::move(ptr)); \
} \
}
CONFIG_VALUE_ARRAY_HANDLER(arflags, std::string)
@@ -258,93 +259,108 @@
bool is_binary_output = target_->IsBinary();
if (what_.empty()) {
- res->SetString("type",
- Target::GetStringForOutputType(target_->output_type()));
- res->SetString(
+ res->SetStringWithoutPathExpansion(
+ "type",
+ Target::GetStringForOutputType(target_->output_type()));
+ res->SetStringWithoutPathExpansion(
"toolchain",
target_->label().GetToolchainLabel().GetUserVisibleName(false));
}
// General target meta variables.
if (what(variables::kVisibility))
- res->Set(variables::kVisibility, target_->visibility().AsValue());
+ res->SetWithoutPathExpansion(variables::kVisibility,
+ target_->visibility().AsValue());
if (what(variables::kTestonly))
- res->SetBoolean(variables::kTestonly, target_->testonly());
+ res->SetBooleanWithoutPathExpansion(variables::kTestonly,
+ target_->testonly());
if (is_binary_output) {
if (what(variables::kCheckIncludes))
- res->SetBoolean(variables::kCheckIncludes, target_->check_includes());
+ res->SetBooleanWithoutPathExpansion(variables::kCheckIncludes,
+ target_->check_includes());
if (what(variables::kAllowCircularIncludesFrom)) {
auto labels = base::MakeUnique<base::ListValue>();
for (const auto& cur : target_->allow_circular_includes_from())
labels->AppendString(cur.GetUserVisibleName(GetToolchainLabel()));
- res->Set(variables::kAllowCircularIncludesFrom, std::move(labels));
+ res->SetWithoutPathExpansion(variables::kAllowCircularIncludesFrom,
+ std::move(labels));
}
}
if (what(variables::kSources) && !target_->sources().empty())
- res->Set(variables::kSources, RenderValue(target_->sources()));
+ res->SetWithoutPathExpansion(variables::kSources,
+ RenderValue(target_->sources()));
if (what(variables::kOutputName) && !target_->output_name().empty())
- res->SetString(variables::kOutputName, target_->output_name());
+ res->SetStringWithoutPathExpansion(variables::kOutputName,
+ target_->output_name());
if (what(variables::kOutputDir) && !target_->output_dir().is_null())
- res->Set(variables::kOutputDir, RenderValue(target_->output_dir()));
+ res->SetWithoutPathExpansion(variables::kOutputDir,
+ RenderValue(target_->output_dir()));
if (what(variables::kOutputExtension) && target_->output_extension_set())
- res->SetString(variables::kOutputExtension, target_->output_extension());
+ res->SetStringWithoutPathExpansion(variables::kOutputExtension,
+ target_->output_extension());
if (what(variables::kPublic)) {
if (target_->all_headers_public())
- res->SetString(variables::kPublic, "*");
+ res->SetStringWithoutPathExpansion(variables::kPublic, "*");
else
- res->Set(variables::kPublic, RenderValue(target_->public_headers()));
+ res->SetWithoutPathExpansion(variables::kPublic,
+ RenderValue(target_->public_headers()));
}
if (what(variables::kInputs) && !target_->inputs().empty())
- res->Set(variables::kInputs, RenderValue(target_->inputs()));
+ res->SetWithoutPathExpansion(variables::kInputs,
+ RenderValue(target_->inputs()));
if (is_binary_output && what(variables::kConfigs) &&
!target_->configs().empty()) {
auto configs = base::MakeUnique<base::ListValue>();
FillInConfigVector(configs.get(), target_->configs().vector());
- res->Set(variables::kConfigs, std::move(configs));
+ res->SetWithoutPathExpansion(variables::kConfigs, std::move(configs));
}
if (what(variables::kPublicConfigs) && !target_->public_configs().empty()) {
auto configs = base::MakeUnique<base::ListValue>();
FillInConfigVector(configs.get(), target_->public_configs());
- res->Set(variables::kPublicConfigs, std::move(configs));
+ res->SetWithoutPathExpansion(variables::kPublicConfigs,
+ std::move(configs));
}
if (what(variables::kAllDependentConfigs) &&
!target_->all_dependent_configs().empty()) {
auto configs = base::MakeUnique<base::ListValue>();
FillInConfigVector(configs.get(), target_->all_dependent_configs());
- res->Set(variables::kAllDependentConfigs, std::move(configs));
+ res->SetWithoutPathExpansion(variables::kAllDependentConfigs,
+ std::move(configs));
}
// Action
if (target_->output_type() == Target::ACTION ||
target_->output_type() == Target::ACTION_FOREACH) {
if (what(variables::kScript))
- res->SetString(variables::kScript,
- target_->action_values().script().value());
+ res->SetStringWithoutPathExpansion(
+ variables::kScript,
+ target_->action_values().script().value());
if (what(variables::kArgs)) {
auto args = base::MakeUnique<base::ListValue>();
for (const auto& elem : target_->action_values().args().list())
args->AppendString(elem.AsString());
- res->Set(variables::kArgs, std::move(args));
+ res->SetWithoutPathExpansion(variables::kArgs, std::move(args));
}
if (what(variables::kDepfile) &&
!target_->action_values().depfile().empty()) {
- res->SetString(variables::kDepfile,
- target_->action_values().depfile().AsString());
+ res->SetStringWithoutPathExpansion(
+ variables::kDepfile,
+ target_->action_values().depfile().AsString());
}
}
@@ -367,7 +383,7 @@
if (what(#name)) { \
ValuePtr ptr = RenderConfigValues<type>(&ConfigValues::name); \
if (ptr) { \
- res->Set(#name, std::move(ptr)); \
+ res->SetWithoutPathExpansion(#name, std::move(ptr)); \
} \
}
CONFIG_VALUE_ARRAY_HANDLER(arflags, std::string)
@@ -388,12 +404,12 @@
}
if (what(variables::kDeps))
- res->Set(variables::kDeps, RenderDeps());
+ res->SetWithoutPathExpansion(variables::kDeps, RenderDeps());
// Runtime deps are special, print only when explicitly asked for and not in
// overview mode.
if (what_.find("runtime_deps") != what_.end())
- res->Set("runtime_deps", RenderRuntimeDeps());
+ res->SetWithoutPathExpansion("runtime_deps", RenderRuntimeDeps());
// libs and lib_dirs are special in that they're inherited. We don't
// currently
@@ -408,7 +424,7 @@
auto libs = base::MakeUnique<base::ListValue>();
for (size_t i = 0; i < all_libs.size(); i++)
libs->AppendString(all_libs[i].value());
- res->Set(variables::kLibs, std::move(libs));
+ res->SetWithoutPathExpansion(variables::kLibs, std::move(libs));
}
}
@@ -418,7 +434,7 @@
auto lib_dirs = base::MakeUnique<base::ListValue>();
for (size_t i = 0; i < all_lib_dirs.size(); i++)
lib_dirs->AppendString(FormatSourceDir(all_lib_dirs[i]));
- res->Set(variables::kLibDirs, std::move(lib_dirs));
+ res->SetWithoutPathExpansion(variables::kLibDirs, std::move(lib_dirs));
}
}
@@ -544,7 +560,7 @@
dict->SetWithoutPathExpansion(source.value(), std::move(list));
}
}
- res->Set("source_outputs", std::move(dict));
+ res->SetWithoutPathExpansion("source_outputs", std::move(dict));
}
void FillInBundle(base::DictionaryValue* res) {
@@ -553,21 +569,27 @@
const Settings* settings = target_->settings();
BundleData::SourceFiles sources;
bundle_data.GetSourceFiles(&sources);
- data->Set("source_files", RenderValue(sources));
- data->SetString("root_dir_output",
- bundle_data.GetBundleRootDirOutput(settings).value());
- data->Set("root_dir", RenderValue(bundle_data.root_dir()));
- data->Set("resources_dir", RenderValue(bundle_data.resources_dir()));
- data->Set("executable_dir", RenderValue(bundle_data.executable_dir()));
- data->Set("plugins_dir", RenderValue(bundle_data.plugins_dir()));
- data->SetString("product_type", bundle_data.product_type());
+ data->SetWithoutPathExpansion("source_files", RenderValue(sources));
+ data->SetStringWithoutPathExpansion(
+ "root_dir_output",
+ bundle_data.GetBundleRootDirOutput(settings).value());
+ data->SetWithoutPathExpansion("root_dir",
+ RenderValue(bundle_data.root_dir()));
+ data->SetWithoutPathExpansion("resources_dir",
+ RenderValue(bundle_data.resources_dir()));
+ data->SetWithoutPathExpansion("executable_dir",
+ RenderValue(bundle_data.executable_dir()));
+ data->SetWithoutPathExpansion("plugins_dir",
+ RenderValue(bundle_data.plugins_dir()));
+ data->SetStringWithoutPathExpansion("product_type",
+ bundle_data.product_type());
auto deps = base::MakeUnique<base::ListValue>();
for (const auto* dep : bundle_data.bundle_deps())
deps->AppendString(dep->label().GetUserVisibleName(GetToolchainLabel()));
- data->Set("deps", std::move(deps));
- res->Set("bundle_data", std::move(data));
+ data->SetWithoutPathExpansion("deps", std::move(deps));
+ res->SetWithoutPathExpansion("bundle_data", std::move(data));
}
void FillInOutputs(base::DictionaryValue* res) {
@@ -576,12 +598,13 @@
for (const auto& elem : target_->action_values().outputs().list())
list->AppendString(elem.AsString());
- res->Set(variables::kOutputs, std::move(list));
+ res->SetWithoutPathExpansion(variables::kOutputs, std::move(list));
} else if (target_->output_type() == Target::CREATE_BUNDLE) {
std::vector<SourceFile> output_files;
target_->bundle_data().GetOutputsAsSourceFiles(target_->settings(),
&output_files);
- res->Set(variables::kOutputs, RenderValue(output_files));
+ res->SetWithoutPathExpansion(variables::kOutputs,
+ RenderValue(output_files));
} else if (target_->output_type() == Target::ACTION_FOREACH ||
target_->output_type() == Target::COPY_FILES) {
const SubstitutionList& outputs = target_->action_values().outputs();
@@ -590,12 +613,13 @@
for (const auto& elem : outputs.list())
patterns->AppendString(elem.AsString());
- res->Set("output_patterns", std::move(patterns));
+ res->SetWithoutPathExpansion("output_patterns", std::move(patterns));
}
std::vector<SourceFile> output_files;
SubstitutionWriter::ApplyListToSources(target_->settings(), outputs,
target_->sources(), &output_files);
- res->Set(variables::kOutputs, RenderValue(output_files));
+ res->SetWithoutPathExpansion(variables::kOutputs,
+ RenderValue(output_files));
} else {
DCHECK(target_->IsBinary());
const Tool* tool =
@@ -609,7 +633,8 @@
output_files_as_source_file.push_back(
output_file.AsSourceFile(target_->settings()->build_settings()));
- res->Set(variables::kOutputs, RenderValue(output_files_as_source_file));
+ res->SetWithoutPathExpansion(variables::kOutputs,
+ RenderValue(output_files_as_source_file));
}
}
diff --git a/tools/gn/json_project_writer.cc b/tools/gn/json_project_writer.cc
index 67c21ee..44576cd 100644
--- a/tools/gn/json_project_writer.cc
+++ b/tools/gn/json_project_writer.cc
@@ -100,19 +100,23 @@
!outputs_value->empty()) {
description->MergeDictionary(outputs.get());
}
- targets->Set(target->label().GetUserVisibleName(default_toolchain_label),
- std::move(description));
+ targets->SetWithoutPathExpansion(
+ target->label().GetUserVisibleName(default_toolchain_label),
+ std::move(description));
}
auto settings = base::MakeUnique<base::DictionaryValue>();
- settings->SetString("root_path", build_settings->root_path_utf8());
- settings->SetString("build_dir", build_settings->build_dir().value());
- settings->SetString("default_toolchain",
- default_toolchain_label.GetUserVisibleName(false));
+ settings->SetStringWithoutPathExpansion("root_path",
+ build_settings->root_path_utf8());
+ settings->SetStringWithoutPathExpansion("build_dir",
+ build_settings->build_dir().value());
+ settings->SetStringWithoutPathExpansion(
+ "default_toolchain",
+ default_toolchain_label.GetUserVisibleName(false));
auto output = base::MakeUnique<base::DictionaryValue>();
- output->Set("targets", std::move(targets));
- output->Set("build_settings", std::move(settings));
+ output->SetWithoutPathExpansion("targets", std::move(targets));
+ output->SetWithoutPathExpansion("build_settings", std::move(settings));
std::string s;
base::JSONWriter::WriteWithOptions(
