-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[build] Allow cross-compiling build-script products for non-Darwin hosts too #36917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,6 +181,10 @@ def convert_to_impl_arguments(self): | |
if args.cross_compile_hosts: | ||
impl_args += [ | ||
"--cross-compile-hosts", " ".join(args.cross_compile_hosts)] | ||
if args.cross_compile_deps_path is not None: | ||
impl_args += [ | ||
"--cross-compile-deps-path=%s" % args.cross_compile_deps_path | ||
] | ||
|
||
if args.test_paths: | ||
impl_args += ["--test-paths", " ".join(args.test_paths)] | ||
|
@@ -664,12 +668,14 @@ def execute(self): | |
self._execute_impl(pipeline, all_hosts, perform_epilogue_opts) | ||
else: | ||
assert(index != last_impl_index) | ||
# Once we have performed our last impl pipeline, we no longer | ||
# support cross compilation. | ||
# | ||
# This just maintains current behavior. | ||
if index > last_impl_index: | ||
self._execute(pipeline, [self.args.host_target]) | ||
non_darwin_cross_compile_hostnames = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @buttaface can you add a comment here (or fix up the comment above) explaining that we support it for non-darwin platforms. I am fine with a follow on commit fixing the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I simply removed the comments above, since it is no longer true. Darwin platforms were already cross-compiling in a different way, and this change adds support for non-Darwin platforms. |
||
target for target in self.args.cross_compile_hosts if not | ||
StdlibDeploymentTarget.get_target_for_name( | ||
target).platform.is_darwin | ||
] | ||
self._execute(pipeline, [self.args.host_target] + | ||
non_darwin_cross_compile_hostnames) | ||
else: | ||
self._execute(pipeline, all_host_names) | ||
|
||
|
@@ -727,6 +733,8 @@ def _execute_impl(self, pipeline, all_hosts, should_run_epilogue_operations): | |
|
||
def _execute(self, pipeline, all_host_names): | ||
for host_target in all_host_names: | ||
if self.args.skip_local_build and host_target == self.args.host_target: | ||
continue | ||
for product_class in pipeline: | ||
# Execute clean, build, test, install | ||
self.execute_product_build_steps(product_class, host_target) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,9 +104,7 @@ def _get_toolchain_path(host_target, product, args): | |
# this logic initially was inside run_build_script_helper | ||
# and was factored out so it can be used in testing as well | ||
|
||
toolchain_path = swiftpm.SwiftPM.get_install_destdir(args, | ||
host_target, | ||
product.build_dir) | ||
toolchain_path = product.host_install_destdir(host_target) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't actually going to work for cross-compiling for non-Darwin so I looked into using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine with this if the tests pass (which it looks like they did). |
||
if platform.system() == 'Darwin': | ||
# The prefix is an absolute path, so concatenate without os.path. | ||
toolchain_path += \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,13 +80,8 @@ def run_build_script_helper(action, host_target, product, args, | |
script_path = os.path.join( | ||
product.source_dir, 'Utilities', 'build-script-helper.py') | ||
|
||
install_destdir = args.install_destdir | ||
if swiftpm.SwiftPM.has_cross_compile_hosts(args): | ||
install_destdir = swiftpm.SwiftPM.get_install_destdir(args, | ||
host_target, | ||
product.build_dir) | ||
toolchain_path = targets.toolchain_path(install_destdir, | ||
args.install_prefix) | ||
install_destdir = product.host_install_destdir(host_target) | ||
toolchain_path = product.native_toolchain_path(host_target) | ||
is_release = product.is_release() | ||
configuration = 'release' if is_release else 'debug' | ||
helper_cmd = [ | ||
|
@@ -110,4 +105,22 @@ def run_build_script_helper(action, host_target, product, args, | |
elif args.enable_tsan: | ||
helper_cmd.extend(['--sanitize', 'thread']) | ||
|
||
if not product.is_darwin_host( | ||
host_target) and product.is_cross_compile_target(host_target): | ||
helper_cmd.extend(['--cross-compile-host', host_target]) | ||
build_toolchain_path = install_destdir + args.install_prefix | ||
resource_dir = '%s/lib/swift' % build_toolchain_path | ||
helper_cmd += [ | ||
'--cross-compile-config', | ||
targets.StdlibDeploymentTarget.get_target_for_name(host_target).platform | ||
.swiftpm_config(args, output_dir=build_toolchain_path, | ||
swift_toolchain=toolchain_path, | ||
resource_path=resource_dir) | ||
] | ||
|
||
if action == 'install' and product.product_name() == "sourcekitlsp": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this also be guarded by the not is_darwin_host since you are trying to not effect the Darwin path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While most of these changes only affect non-Darwin and are guarded by such checks, there are a few changes that are applied generally, ie for Darwin too, which I called out separately in my notes for the first commit and the second commit:
This was needed because non-Darwin uses a different install prefix for cross-compilation hosts, so I submitted pulls for those products' build scripts to accept an install prefix or actually start using the prefix passed in, which this pull now passes in for all platforms. These configuration changes that affect Darwin too appear to be working fine. |
||
helper_cmd.extend([ | ||
'--prefix', install_destdir + args.install_prefix | ||
]) | ||
|
||
shell.call(helper_cmd) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,18 +195,23 @@ def install_toolchain_path(self, host_target): | |
"""toolchain_path() -> string | ||
|
||
Returns the path to the toolchain that is being created as part of this | ||
build, or to a native prebuilt toolchain that was passed in. | ||
build | ||
""" | ||
if self.args.native_swift_tools_path is not None: | ||
return os.path.split(self.args.native_swift_tools_path)[0] | ||
finagolfin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
install_destdir = self.args.install_destdir | ||
if self.args.cross_compile_hosts: | ||
build_root = os.path.dirname(self.build_dir) | ||
install_destdir = '%s/intermediate-install/%s' % (build_root, host_target) | ||
if self.is_darwin_host(host_target): | ||
install_destdir = self.host_install_destdir(host_target) | ||
else: | ||
install_destdir = os.path.join(install_destdir, self.args.host_target) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could only replace the Darwin line above with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Darwin places all targets together and then |
||
return targets.toolchain_path(install_destdir, | ||
self.args.install_prefix) | ||
|
||
def native_toolchain_path(self, host_target): | ||
if self.args.native_swift_tools_path is not None: | ||
return os.path.split(self.args.native_swift_tools_path)[0] | ||
else: | ||
return self.install_toolchain_path(host_target) | ||
|
||
def is_darwin_host(self, host_target): | ||
return host_target.startswith("macosx") or \ | ||
host_target.startswith("iphone") or \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,13 +86,8 @@ def run_build_script_helper(action, host_target, product, args): | |
script_path = os.path.join( | ||
product.source_dir, 'Utilities', 'build-script-helper.py') | ||
|
||
install_destdir = args.install_destdir | ||
if swiftpm.SwiftPM.has_cross_compile_hosts(args): | ||
install_destdir = swiftpm.SwiftPM.get_install_destdir(args, | ||
host_target, | ||
product.build_dir) | ||
toolchain_path = targets.toolchain_path(install_destdir, | ||
args.install_prefix) | ||
install_destdir = product.host_install_destdir(host_target) | ||
toolchain_path = product.native_toolchain_path(host_target) | ||
|
||
# Pass Dispatch directory down if we built it | ||
dispatch_build_dir = os.path.join( | ||
|
@@ -134,10 +129,26 @@ def run_build_script_helper(action, host_target, product, args): | |
] | ||
# Pass Cross compile host info | ||
if swiftpm.SwiftPM.has_cross_compile_hosts(args): | ||
helper_cmd += ['--cross-compile-hosts'] | ||
for cross_compile_host in args.cross_compile_hosts: | ||
helper_cmd += [cross_compile_host] | ||
if product.is_darwin_host(host_target): | ||
helper_cmd += ['--cross-compile-hosts'] | ||
for cross_compile_host in args.cross_compile_hosts: | ||
helper_cmd += [cross_compile_host] | ||
elif product.is_cross_compile_target(host_target): | ||
helper_cmd += ['--cross-compile-hosts', host_target] | ||
build_toolchain_path = install_destdir + args.install_prefix | ||
resource_dir = '%s/lib/swift' % build_toolchain_path | ||
helper_cmd += [ | ||
'--cross-compile-config', | ||
finagolfin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
targets.StdlibDeploymentTarget.get_target_for_name( | ||
host_target).platform.swiftpm_config( | ||
args, output_dir=build_toolchain_path, | ||
swift_toolchain=toolchain_path, resource_path=resource_dir)] | ||
if args.verbose_build: | ||
helper_cmd.append('--verbose') | ||
|
||
if action == 'install': | ||
helper_cmd += [ | ||
'--prefix', install_destdir + args.install_prefix | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as my earlier question about --prefix. I just want to understand what the effect is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same answer as indexstoredb above. |
||
] | ||
|
||
shell.call(helper_cmd) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
from . import swift | ||
from . import xctest | ||
from .. import shell | ||
from ..targets import StdlibDeploymentTarget | ||
|
||
|
||
class SwiftPM(product.Product): | ||
|
@@ -44,7 +45,8 @@ def should_build(self, host_target): | |
def run_bootstrap_script(self, action, host_target, additional_params=[]): | ||
script_path = os.path.join( | ||
self.source_dir, 'Utilities', 'bootstrap') | ||
toolchain_path = self.install_toolchain_path(host_target) | ||
|
||
toolchain_path = self.native_toolchain_path(host_target) | ||
swiftc = os.path.join(toolchain_path, "bin", "swiftc") | ||
|
||
# FIXME: We require llbuild build directory in order to build. Is | ||
|
@@ -92,9 +94,22 @@ def run_bootstrap_script(self, action, host_target, additional_params=[]): | |
|
||
# Pass Cross compile host info | ||
if self.has_cross_compile_hosts(self.args): | ||
helper_cmd += ['--cross-compile-hosts'] | ||
for cross_compile_host in self.args.cross_compile_hosts: | ||
helper_cmd += [cross_compile_host] | ||
if self.is_darwin_host(host_target): | ||
helper_cmd += ['--cross-compile-hosts'] | ||
for cross_compile_host in self.args.cross_compile_hosts: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for a loop here if we change the SPM bootstrap flag to |
||
helper_cmd += [cross_compile_host] | ||
elif self.is_cross_compile_target(host_target): | ||
helper_cmd += ['--cross-compile-hosts', host_target, | ||
'--skip-cmake-bootstrap'] | ||
build_toolchain_path = self.host_install_destdir( | ||
host_target) + self.args.install_prefix | ||
resource_dir = '%s/lib/swift' % build_toolchain_path | ||
helper_cmd += [ | ||
'--cross-compile-config', | ||
StdlibDeploymentTarget.get_target_for_name(host_target).platform | ||
.swiftpm_config(self.args, output_dir=build_toolchain_path, | ||
swift_toolchain=toolchain_path, | ||
resource_path=resource_dir)] | ||
|
||
helper_cmd.extend(additional_params) | ||
|
||
|
@@ -122,18 +137,8 @@ def should_install(self, host_target): | |
def has_cross_compile_hosts(self, args): | ||
return args.cross_compile_hosts | ||
|
||
@classmethod | ||
def get_install_destdir(self, args, host_target, build_dir): | ||
install_destdir = args.install_destdir | ||
if self.has_cross_compile_hosts(args): | ||
build_root = os.path.dirname(build_dir) | ||
install_destdir = '%s/intermediate-install/%s' % (build_root, host_target) | ||
return install_destdir | ||
|
||
def install(self, host_target): | ||
install_destdir = self.get_install_destdir(self.args, | ||
host_target, | ||
self.build_dir) | ||
install_destdir = self.host_install_destdir(host_target) | ||
install_prefix = install_destdir + self.args.install_prefix | ||
|
||
self.run_bootstrap_script('install', host_target, [ | ||
|
Uh oh!
There was an error while loading. Please reload this page.