[xcode] Wrap ninja invocation for Xcode New Build System
The New Build System shipped with Xcode 10+ expects the paths
in error messages to be absolute even when invoking a custom
build script target.
Wrap the invocation of ninja in a python script that uses
regular expressions to convert the relative paths to absolute
paths.
Remove deprecated parameter --ninja-extra-args as it is hard
to support with python script (it would require parsing the
string and interpret it as a shell would do).
Bug: chromium:1094890
Change-Id: I893128f0c6744f98c8c89f1a807c83400be26545
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9080
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/command_gen.cc b/src/gn/command_gen.cc
index 0180cd1..b3f0633 100644
--- a/src/gn/command_gen.cc
+++ b/src/gn/command_gen.cc
@@ -244,7 +244,6 @@
command_line->GetSwitchValueASCII(kSwitchXcodeProject),
command_line->GetSwitchValueASCII(kSwitchRootTarget),
command_line->GetSwitchValueASCII(kSwitchNinjaExecutable),
- command_line->GetSwitchValueASCII(kSwitchNinjaExtraArgs),
command_line->GetSwitchValueASCII(kSwitchFilters),
XcodeBuildSystem::kLegacy,
};
diff --git a/src/gn/xcode_object.cc b/src/gn/xcode_object.cc
index eb3bd64..b56f6a9 100644
--- a/src/gn/xcode_object.cc
+++ b/src/gn/xcode_object.cc
@@ -994,7 +994,7 @@
PrintProperty(out, rules, "name", name_);
PrintProperty(out, rules, "outputPaths", EmptyPBXObjectVector());
PrintProperty(out, rules, "runOnlyForDeploymentPostprocessing", 0u);
- PrintProperty(out, rules, "shellPath", "/bin/sh");
+ PrintProperty(out, rules, "shellPath", "/usr/bin/python3");
PrintProperty(out, rules, "shellScript", shell_script_);
PrintProperty(out, rules, "showEnvVarsInLog", 0u);
out << indent_str << "};\n";
diff --git a/src/gn/xcode_writer.cc b/src/gn/xcode_writer.cc
index 0daf1c5..0060545 100644
--- a/src/gn/xcode_writer.cc
+++ b/src/gn/xcode_writer.cc
@@ -19,6 +19,7 @@
#include "base/stl_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
#include "gn/args.h"
#include "gn/build_settings.h"
#include "gn/builder.h"
@@ -40,6 +41,60 @@
namespace {
+// This is the template of the script used to build the target. It invokes
+// ninja (supporting --ninja-executable parameter), parsing ninja's output
+// using a regular expression looking for relative path to the source root
+// from root_build_dir that are at the start of a path and converting them
+// to absolute paths (use str.replace(rel_root_src, abs_root_src) would be
+// simpler but would fail if rel_root_src is present multiple time in the
+// path).
+const char kBuildScriptTemplate[] = R"(
+import re
+import os
+import subprocess
+import sys
+
+rel_root_src = '%s'
+abs_root_src = os.path.abspath(rel_root_src) + '/'
+
+build_target = '%s'
+ninja_binary = '%s'
+ninja_params = [ '-C', '.' ]
+
+%s
+
+if build_target:
+ ninja_params.append(build_target)
+ print('Compile "' + build_target + '" via ninja')
+else:
+ print('Compile "all" via ninja')
+
+process = subprocess.Popen(
+ [ ninja_binary ] + ninja_params,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ universal_newlines=True,
+ encoding='utf-8',
+ env=environ)
+
+pattern = re.compile('(?<!/)' + re.escape(rel_root_src))
+
+for line in iter(process.stdout.readline, ''):
+ while True:
+ match = pattern.search(line)
+ if not match:
+ break
+ span = match.span()
+ print(line[:span[0]], end='')
+ print(abs_root_src, end='')
+ line = line[span[1]:]
+ print(line, flush=True, end='')
+
+process.wait()
+
+sys.exit(process.returncode)
+)";
+
enum TargetOsType {
WRITER_TARGET_OS_IOS,
WRITER_TARGET_OS_MACOS,
@@ -84,39 +139,31 @@
return ninja_executable.empty() ? "ninja" : ninja_executable;
}
+std::string ComputeScriptEnviron(base::Environment* environment) {
+ std::stringstream buffer;
+ buffer << "environ = {}";
+ for (const auto& variable : kSafeEnvironmentVariables) {
+ buffer << "\nenviron['" << variable.name << "'] = ";
+ if (variable.capture_at_generation) {
+ std::string value;
+ environment->GetVar(variable.name, &value);
+ buffer << "'" << value << "'";
+ } else {
+ buffer << "os.environ.get('" << variable.name << "', '')";
+ }
+ }
+ return buffer.str();
+}
+
std::string GetBuildScript(const std::string& target_name,
const std::string& ninja_executable,
- const std::string& ninja_extra_args,
+ const std::string& root_src_dir,
base::Environment* environment) {
- std::stringstream script;
- script << "echo note: \"Compile and copy " << target_name << " via ninja\"\n"
- << "exec ";
-
- // Launch ninja with a sanitized environment (Xcode sets many environment
- // variable overridding settings, including the SDK, thus breaking hermetic
- // build).
- script << "env -i ";
- for (const auto& variable : kSafeEnvironmentVariables) {
- 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 << GetNinjaExecutable(ninja_executable) << " -C .";
- if (!ninja_extra_args.empty())
- script << " " << ninja_extra_args;
- if (!target_name.empty())
- script << " " << target_name;
- script << "\nexit 1\n";
- return script.str();
+ std::string environ_script = ComputeScriptEnviron(environment);
+ std::string ninja = GetNinjaExecutable(ninja_executable);
+ return base::StringPrintf(kBuildScriptTemplate, root_src_dir.c_str(),
+ target_name.c_str(), ninja.c_str(),
+ environ_script.c_str());
}
bool IsApplicationTarget(const Target* target) {
@@ -663,10 +710,11 @@
bool XcodeProject::AddTargetsFromBuilder(const Builder& builder, Err* err) {
std::unique_ptr<base::Environment> env(base::Environment::Create());
- project_.AddAggregateTarget(
- "All",
- GetBuildScript(options_.root_target_name, options_.ninja_executable,
- options_.ninja_extra_args, env.get()));
+ const std::string root_src_dir =
+ RebasePath("//", build_settings_->build_dir());
+ project_.AddAggregateTarget("All", GetBuildScript(options_.root_target_name,
+ options_.ninja_executable,
+ root_src_dir, env.get()));
const std::optional<std::vector<const Target*>> targets =
GetTargetsFromBuilder(builder, err);
@@ -898,14 +946,15 @@
output_dir = RebasePath(output_dir, build_settings_->build_dir());
}
+ const std::string root_src_dir =
+ RebasePath("//", build_settings_->build_dir());
return project_.AddNativeTarget(
target->label().name(), "compiled.mach-o.executable",
target->output_name().empty() ? target->label().name()
: target->output_name(),
- "com.apple.product-type.tool",
- output_dir,
+ "com.apple.product-type.tool", output_dir,
GetBuildScript(target->label().name(), options_.ninja_executable,
- options_.ninja_extra_args, env));
+ root_src_dir, env));
}
PBXNativeTarget* XcodeProject::AddBundleTarget(const Target* target,
@@ -933,12 +982,13 @@
.GetBundleDir(target->settings())
.value(),
build_settings_->build_dir());
+ const std::string root_src_dir =
+ RebasePath("//", build_settings_->build_dir());
return project_.AddNativeTarget(
pbxtarget_name, std::string(), target_output_name,
- target->bundle_data().product_type(),
- output_dir,
- GetBuildScript(pbxtarget_name, options_.ninja_executable,
- options_.ninja_extra_args, env),
+ target->bundle_data().product_type(), output_dir,
+ GetBuildScript(pbxtarget_name, options_.ninja_executable, root_src_dir,
+ env),
xcode_extra_attributes);
}
diff --git a/src/gn/xcode_writer.h b/src/gn/xcode_writer.h
index b45b606..f934275 100644
--- a/src/gn/xcode_writer.h
+++ b/src/gn/xcode_writer.h
@@ -38,9 +38,6 @@
// Name of the ninja executable. Defaults to "ninja" if empty.
std::string ninja_executable;
- // Extra parameters to pass to ninja. Deprecated.
- std::string ninja_extra_args;
-
// If specified, should be a semicolon-separated list of label patterns.
// It will be used to filter the list of targets generated in the project
// (in the same way that the other filtering is done, source and header
