Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(684)

Unified Diff: tools/gn/xcode_writer.cc

Issue 2544803002: [GN] Sanitize environment variables when running ninja from Xcode. (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/gn/xcode_writer.cc
diff --git a/tools/gn/xcode_writer.cc b/tools/gn/xcode_writer.cc
index 09f62dbbbe5b282934c8c1c3cc1d709c117e889c..6e5042865aa1dc746382e68c191f76d33edb9dd1 100644
--- a/tools/gn/xcode_writer.cc
+++ b/tools/gn/xcode_writer.cc
@@ -31,6 +31,16 @@
namespace {
+struct SafeEnvironmentVariableInfo {
+ const char* name;
+ bool capture_at_generation;
+};
+
+SafeEnvironmentVariableInfo kSafeEnvironmentVariables[] = {
+ {"HOME", true}, {"LANG", true}, {"PATH", true},
Dirk Pranke 2016/12/01 20:53:22 nit: weird indentation.
+ {"USER", true}, {"TMPDIR", false},
+};
+
XcodeWriter::TargetOsType GetTargetOs(const Args& args) {
const Value* target_os_value = args.GetArgOverride(variables::kTargetOs);
if (target_os_value) {
@@ -43,13 +53,31 @@ XcodeWriter::TargetOsType GetTargetOs(const Args& args) {
}
std::string GetBuildScript(const std::string& target_name,
- const std::string& build_path,
- const std::string& ninja_extra_args) {
+ const std::string& ninja_extra_args,
+ base::Environment* environment) {
std::stringstream script;
script << "echo note: \"Compile and copy " << target_name << " via ninja\"\n"
<< "exec ";
- if (!build_path.empty())
- script << "env PATH=\"" << build_path << "\" ";
+
+ // Launch ninja with a sanitized environment (Xcode sets many environment
+ // variable overridding settings, including the SDK, thus breaking hermetic
+ // build).
+ script << "env -i ";
+ for (size_t i = 0; i < arraysize(kSafeEnvironmentVariables); ++i) {
Dirk Pranke 2016/12/01 20:53:22 Can you replace this with a range-based loop? I.e.
+ const auto& variable = kSafeEnvironmentVariables[i];
+ script << variable.name << "=\"";
+
+ std::string value;
+ if (variable.capture_at_generation)
+ environment->GetVar(variable.name, &value);
+
+ if (!value.empty())
+ script << value;
+ else
+ script << "$" << variable.name;
+ script << "\" ";
+ }
+
script << "ninja -C .";
if (!ninja_extra_args.empty())
script << " " << ninja_extra_args;
@@ -253,10 +281,9 @@ void XcodeWriter::CreateProductsProject(
std::string build_path;
std::unique_ptr<base::Environment> env(base::Environment::Create());
- env->GetVar("PATH", &build_path);
main_project->AddAggregateTarget(
- "All", GetBuildScript(root_target, build_path, ninja_extra_args));
+ "All", GetBuildScript(root_target, ninja_extra_args, env.get()));
for (const Target* target : targets) {
switch (target->output_type()) {
@@ -269,8 +296,8 @@ void XcodeWriter::CreateProductsProject(
target->output_name().empty() ? target->label().name()
: target->output_name(),
"com.apple.product-type.tool",
- GetBuildScript(target->label().name(), build_path,
- ninja_extra_args));
+ GetBuildScript(target->label().name(), ninja_extra_args,
+ env.get()));
break;
case Target::CREATE_BUNDLE:
@@ -284,8 +311,8 @@ void XcodeWriter::CreateProductsProject(
.value(),
build_settings->build_dir()),
target->bundle_data().product_type(),
- GetBuildScript(target->label().name(), build_path,
- ninja_extra_args));
+ GetBuildScript(target->label().name(), ninja_extra_args,
+ env.get()));
break;
default:
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698