Skip to content

[build-script] Compute targets in build-script #2880

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 9, 2016
Merged

[build-script] Compute targets in build-script #2880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c03943c
[build-script] Lift `--cross-compile-hosts` to build-script level.
ddunbar Jun 3, 2016
ca2d64a
[build-script] Add infrastructure for passing host-specific variables.
ddunbar Jun 3, 2016
bf75c8f
[build-script] Create explicit Platform and Target types.
ddunbar Jun 4, 2016
6df6ae7
[build-script] Add a couple more options at the `build-script` layer.
ddunbar Jun 4, 2016
f407cdf
[swift_build_support.targets] Add some additional metadata.
ddunbar Jun 4, 2016
bb9fc67
[build-script] Switch over to computing targets in `build-script`.
ddunbar Jun 4, 2016
d515078
[build-script] Several PEP8 fixes.
ddunbar Jun 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions utils/SwiftBuildSupport.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,22 +151,22 @@ def get_preset_options(substitutions, preset_file_names, preset_name):
from swift_build_support.targets import StdlibDeploymentTarget
for sdk in sdks_to_configure:
if sdk == "OSX":
tgts += StdlibDeploymentTarget.OSX.allArchs
tgts += StdlibDeploymentTarget.OSX.targets
elif sdk == "IOS":
tgts += StdlibDeploymentTarget.iOS.allArchs
tgts += StdlibDeploymentTarget.iOS.targets
elif sdk == "IOS_SIMULATOR":
tgts += StdlibDeploymentTarget.iOSSimulator.allArchs
tgts += StdlibDeploymentTarget.iOSSimulator.targets
elif sdk == "TVOS":
tgts += StdlibDeploymentTarget.AppleTV.allArchs
tgts += StdlibDeploymentTarget.AppleTV.targets
elif sdk == "TVOS_SIMULATOR":
tgts += StdlibDeploymentTarget.AppleTVSimulator.allArchs
tgts += StdlibDeploymentTarget.AppleTVSimulator.targets
elif sdk == "WATCHOS":
tgts += StdlibDeploymentTarget.AppleWatch.allArchs
tgts += StdlibDeploymentTarget.AppleWatch.targets
elif sdk == "WATCHOS_SIMULATOR":
tgts += StdlibDeploymentTarget.AppleWatchSimulator.allArchs
tgts += StdlibDeploymentTarget.AppleWatchSimulator.targets

build_script_opts.append("--stdlib-deployment-targets=" +
" ".join(tgts))
" ".join([tgt.name for tgt in tgts]))
# Filter the swift-sdks parameter
build_script_impl_opts = [opt for opt in build_script_impl_opts
if not opt.startswith("--swift-sdks")]
Expand Down
277 changes: 267 additions & 10 deletions utils/build-script
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ from swift_build_support.cmake import CMake # noqa (E402)
import swift_build_support.workspace # noqa (E402)


def call_without_sleeping(command, dry_run=False):
def call_without_sleeping(command, env=None, dry_run=False):
"""
Execute a command during which system sleep is disabled.

Expand All @@ -62,7 +62,119 @@ def call_without_sleeping(command, dry_run=False):
# Don't mutate the caller's copy of the arguments.
command = ["caffeinate"] + list(command)

shell.call(command, dry_run=dry_run, echo=False)
shell.call(command, env=env, dry_run=dry_run)


class HostSpecificConfiguration(object):
"""Configuration information for an individual host."""

def __init__(self, host_target, invocation):
"""Initialize for the given `host_target`."""

# Compute the set of deployment targets to configure/build.
args = invocation.args
if host_target == args.host_target:
# This host is the user's desired product, so honor the requested
# set of targets to configure/build.
stdlib_targets_to_configure = args.stdlib_deployment_targets
if "all" in args.build_stdlib_deployment_targets:
stdlib_targets_to_build = set(stdlib_targets_to_configure)
else:
stdlib_targets_to_build = set(
args.build_stdlib_deployment_targets).intersect(
set(args.stdlib_deployment_targets))
else:
# Otherwise, this is a host we are building as part of
# cross-compiling, so we only need the target itself.
stdlib_targets_to_configure = [host_target]
stdlib_targets_to_build = set(stdlib_targets_to_configure)

# Compute the lists of **CMake** targets for each use case (configure
# vs. build vs. run) and the SDKs to configure with.
self.sdks_to_configure = set()
self.swift_stdlib_build_targets = []
self.swift_test_run_targets = []
self.swift_benchmark_build_targets = []
self.swift_benchmark_run_targets = []
for deployment_target_name in stdlib_targets_to_configure:
# Get the target object.
deployment_target = StdlibDeploymentTarget.get_target_for_name(
deployment_target_name)
if deployment_target is None:
diagnostics.fatal("unknown target: %r" % (
deployment_target_name,))

# Add the SDK to use.
deployment_platform = deployment_target.platform
self.sdks_to_configure.add(deployment_platform.sdk_name)

# If we aren't actually building this target (only configuring
# it), do nothing else.
if deployment_target_name not in stdlib_targets_to_build:
continue

# Compute which actions are desired.
build = (
deployment_platform not in invocation.platforms_to_skip_build)
test = (
deployment_platform not in invocation.platforms_to_skip_test)
test_host_only = None
build_benchmark = build and deployment_target.supports_benchmark
# FIXME: Note, `build-script-impl` computed a property here
# w.r.t. testing, but it was actually unused.

# For platforms which normally require a connected device to
# test, the default behavior is to run tests that only require
# the host (i.e., they do not attempt to execute).
if deployment_platform.is_darwin and \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place we use it, right? Seems easy enough to make generalised. I mean, if this were written in Swift, the compiler would go bananas at you checking for the existence of instance variables based solely on another instance variable. We should set a good example!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean, are you talking about the is_darwin flag? Yes it is the only use currently, but I believe there are other places we will need it as the build-script-impl merging progresses.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only have comparable checks in two places:

  • if a target has darwin symbols (i.e. whether or not we should dsymutil -- but we could generalise that as an executable_format and check for Mach-O instead; it's technically more accurate and that's useful information for cross-compilers anyway. I built llvm-dsymutil for linux, and if I had any Mach-Os on there I needed to strip, I'd be using it.)
  • Whether or not to lipo. Could also go under the Mach-O heading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, however, doesn't include examples like the one I pasted above, where we have code duplication that is doing the same thing for three different Darwin embedded platforms, but hadn't been factored into common code.

IMHO, factoring it into common code (even using is_darwin) is monotonic improvement that then makes it easier to generalize to a non-Darwin attribute. So I see this as a step in the direction you want...

Copy link
Contributor

@karwa karwa Jun 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wasn't a great example; host tests are soon not going to be Darwin-exclusive. The fact that they are exclusive isn't actually inherent to Darwin; it's a limitation of what we have implemented right now. That's actually a great example of why not to hide everything behind is_darwin-esque magic IMO.

Maybe it's bikeshedding, but this is how it starts. Once that is_darwin flag is in there, it'll be used for everything.

deployment_platform.is_embedded and \
not deployment_platform.is_simulator:
if deployment_platform not in \
invocation.platforms_to_skip_test_host:
test_host_only = True
test = True
else:
test = False

name = deployment_target.name
if build:
# Validation and long tests require building the full standard
# library, whereas the other targets can build a slightly
# smaller subset which is faster to build.
if args.build_swift_stdlib_unittest_extra or \
args.validation_test or args.long_test:
self.swift_stdlib_build_targets.append(
"swift-stdlib-" + name)
else:
self.swift_stdlib_build_targets.append(
"swift-test-stdlib-" + name)
if build_benchmark:
self.swift_benchmark_build_targets.append(
"swift-benchmark-" + name)
# FIXME: This probably should respect `args.benchmark`, but
# a typo in build-script-impl meant we always would do this.
self.swift_benchmark_run_targets.append(
"check-swift-benchmark-" + name)
if test:
if test_host_only:
suffix = "-non-executable"
else:
suffix = ""
subset_suffix = ""
if args.validation_test and args.long_test:
subset_suffix = "-all"
elif args.validation_test:
subset_suffix = "-validation"
elif args.long_test:
subset_suffix = "-only_long"
else:
subset_suffix = ""
self.swift_test_run_targets.append("check-swift{}{}-{}".format(
subset_suffix, suffix, name))
if args.test_optimized and not test_host_only:
self.swift_test_run_targets.append(
"check-swift{}-optimize-{}".format(
subset_suffix, name))


class BuildScriptInvocation(object):
Expand Down Expand Up @@ -291,6 +403,79 @@ class BuildScriptInvocation(object):
source_root=SWIFT_SOURCE_ROOT,
build_root=os.path.join(SWIFT_BUILD_ROOT, args.build_subdir))

# Compute derived information from the arguments.
#
# FIXME: We should move the platform-derived arguments to be entirely
# data driven, so that we can eliminate this code duplication and just
# iterate over all supported platforms.

self.platforms_to_skip_build = set()
if args.skip_build_linux:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.Linux)
if args.skip_build_freebsd:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.FreeBSD)
if args.skip_build_cygwin:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.Cygwin)
if args.skip_build_osx:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.OSX)
if args.skip_build_ios_device:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.iOS)
if args.skip_build_ios_simulator:
self.platforms_to_skip_build.add(
StdlibDeploymentTarget.iOSSimulator)
if args.skip_build_tvos_device:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.AppleTV)
if args.skip_build_tvos_simulator:
self.platforms_to_skip_build.add(
StdlibDeploymentTarget.AppleTVSimulator)
if args.skip_build_watchos_device:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.AppleWatch)
if args.skip_build_watchos_simulator:
self.platforms_to_skip_build.add(
StdlibDeploymentTarget.AppleWatchSimulator)
if args.skip_build_android:
self.platforms_to_skip_build.add(StdlibDeploymentTarget.Android)

self.platforms_to_skip_test = set()
if args.skip_test_linux:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.Linux)
if args.skip_test_freebsd:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.FreeBSD)
if args.skip_test_cygwin:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.Cygwin)
if args.skip_test_osx:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.OSX)
if args.skip_test_ios_host:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.iOS)
if args.skip_test_ios_simulator:
self.platforms_to_skip_test.add(
StdlibDeploymentTarget.iOSSimulator)
if args.skip_test_tvos_host:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.AppleTV)
if args.skip_test_tvos_simulator:
self.platforms_to_skip_test.add(
StdlibDeploymentTarget.AppleTVSimulator)
if args.skip_test_watchos_host:
self.platforms_to_skip_test.add(StdlibDeploymentTarget.AppleWatch)
if args.skip_test_watchos_simulator:
self.platforms_to_skip_test.add(
StdlibDeploymentTarget.AppleWatchSimulator)
# We never allow testing Android, currently.
#
# FIXME: Allow Android host tests to be enabled/disabled by the build
# script.
self.platforms_to_skip_test.add(StdlibDeploymentTarget.Android)

self.platforms_to_skip_test_host = set()
if args.skip_test_ios_host:
self.platforms_to_skip_test_host.add(StdlibDeploymentTarget.iOS)
if args.skip_test_tvos_host:
self.platforms_to_skip_test_host.add(
StdlibDeploymentTarget.AppleTV)
if args.skip_test_watchos_host:
self.platforms_to_skip_test_host.add(
StdlibDeploymentTarget.AppleWatch)

def initialize_runtime_environment(self):
"""Change the program environment for building."""

Expand Down Expand Up @@ -322,10 +507,10 @@ class BuildScriptInvocation(object):
self.toolchain.ninja = ninja_build.ninja_bin_path

def convert_to_impl_arguments(self):
"""convert_to_impl_arguments() -> args
"""convert_to_impl_arguments() -> (env, args)

Convert the invocation to a list of arguments suitable for invoking
`build-script-impl`.
Convert the invocation to an environment and list of arguments suitable
for invoking `build-script-impl`.
"""

# Create local shadows, for convenience.
Expand Down Expand Up @@ -374,6 +559,14 @@ class BuildScriptInvocation(object):
pipes.quote(arg) for arg in cmake.build_args()),
]

if args.build_stdlib_deployment_targets:
impl_args += [
"--build-stdlib-deployment-targets", " ".join(
args.build_stdlib_deployment_targets)]
if args.cross_compile_hosts:
impl_args += [
"--cross-compile-hosts", " ".join(args.cross_compile_hosts)]

if toolchain.ninja:
impl_args += ["--ninja-bin=%s" % toolchain.ninja]
if args.distcc:
Expand Down Expand Up @@ -415,6 +608,8 @@ class BuildScriptInvocation(object):
impl_args += ["--skip-build-libdispatch"]
if not args.build_swiftpm:
impl_args += ["--skip-build-swiftpm"]
if args.build_swift_stdlib_unittest_extra:
impl_args += ["--build-swift-stdlib-unittest-extra"]

if args.skip_build_linux:
impl_args += ["--skip-build-linux"]
Expand Down Expand Up @@ -536,7 +731,48 @@ class BuildScriptInvocation(object):
if args.dry_run:
impl_args += ["--dry-run"]

return impl_args
# Compute the set of host-specific variables, which we pass through to
# the build script via environment variables.
host_specific_variables = self.compute_host_specific_variables()
impl_env = {}
for (host_target, options) in host_specific_variables.items():
for (name, value) in options.items():
# We mangle into an environment variable we can easily evaluate
# from the `build-script-impl`.
impl_env["HOST_VARIABLE_{}__{}".format(
host_target.replace("-", "_"), name)] = value

return (impl_env, impl_args)

def compute_host_specific_variables(self):
"""compute_host_specific_variables(args) -> dict

Compute the host-specific options, organized as a dictionary keyed by
host of options.
"""

args = self.args

options = {}
for host_target in [args.host_target] + args.cross_compile_hosts:
# Compute the host specific configuration.
config = HostSpecificConfiguration(host_target, self)

# Convert into `build-script-impl` style variables.
options[host_target] = {
"SWIFT_SDKS": " ".join(sorted(
config.sdks_to_configure)),
"SWIFT_STDLIB_TARGETS": " ".join(
config.swift_stdlib_build_targets),
"SWIFT_BENCHMARK_TARGETS": " ".join(
config.swift_benchmark_build_targets),
"SWIFT_RUN_BENCHMARK_TARGETS": " ".join(
config.swift_benchmark_run_targets),
"SWIFT_TEST_TARGETS": " ".join(
config.swift_test_run_targets),
}

return options


# Main entry point for the preset mode.
Expand Down Expand Up @@ -814,13 +1050,27 @@ details of the setups of other systems or automated environments.""")
help="The host target. LLVM, Clang, and Swift will be built for this "
"target. The built LLVM and Clang will be used to compile Swift "
"for the cross-compilation targets.",
default=StdlibDeploymentTarget.host_target())
default=StdlibDeploymentTarget.host_target().name)
targets_group.add_argument(
"--cross-compile-hosts",
help="A space separated list of targets to cross-compile host Swift "
"tools for. Can be used multiple times.",
action=arguments.action.concat, type=arguments.type.shell_split,
default=[])
stdlib_targets = StdlibDeploymentTarget.default_stdlib_deployment_targets()
targets_group.add_argument(
"--stdlib-deployment-targets",
help="list of targets to compile or cross-compile the Swift standard "
"library for. %(default)s by default.",
nargs="*",
default=StdlibDeploymentTarget.default_stdlib_deployment_targets())
default=[
target.name
for target in stdlib_targets])
targets_group.add_argument(
"--build-stdlib-deployment-targets",
Copy link
Contributor

@gribozavr gribozavr Jun 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have a consistent model (from the user's perspective) between cross-compiling the tools and the library. What about --cross-compile-tools-for-targets and --cross-compile-stdlib-for-targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just porting over existing names from build-script-impl, can we separate out renames/behavior changes? Or are you trying to ensure it gets cleaned up as we port?

Copy link
Contributor

@gribozavr gribozavr Jun 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be OK with adding these as FIXMEs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm not even sure I understand the comment. This flag is just a filter on --stdlib-deployment-targets... what makes those names more consistent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, re-reading now I see that my initial message was confusing. What I meant is:

--cross-compile-hosts       => --cross-compile-tools-for-hosts
--stdlib-deployment-targets => --cross-compile-stdlib-for-targets

Does this make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I take it we're not supporting cross-compiling for NxM hosts and targets?

That is, --cross-compile-stdlib-for-targets might be confused with some future option to specify which stdlibs are compiled for the cross-compiled hosts (e.g. I might be on OSX, building a linux-armv7 compiler capable of targeting itself and linux-x86_64). Currently, cross-compiled hosts can only target themselves. If you want anything else, you need to build it for the build machine and manually copy it over.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I take it we're not supporting cross-compiling for NxM hosts and targets?

I can totally see adding support for building N cross-stdlibs in one run, but I would prefer to limit ourselves to cross-compiling just one compiler, which I think is the common case. If you think cross-compiling multiple compilers would be easy to support, I don't have objections in principle -- especially if there is a compelling usecase.

Copy link
Contributor

@karwa karwa Jun 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, actually I was just thinking maybe we shouldn't concat these? How about this for an interface:

--cross-compile-host linux-armv7 \
--cross-compile-stdlib-targets linux-armv7 linux-x86_64 \
--cross-compile-host linux-x86_64 \
--cross-compile-stdlib-targets linux-x86_64 linux-armv7 android-armv7 \

If we get a list of lists, we can match up the host with the list of stdlib targets. That would be very cool, and opens up some possibilities:

How about we replace it with a verb? Merge --stdlib-deployment-targets with --cross-compile-stdlib-deployment-targets, in to --target, so you can go:

./build-script -R \
--target macos-x86_64 \  # First time, it's for the local host
--cross-compile-for linux-armv7 \
--target linux-armv7 linux-x86_64 \ # now it's for args.cross_compile_for[idx-1]
--cross-compile-for linux-x86_64 \
--target linux-x86_64 linux-armv7 android-armv7 \

This would work nicely with the explicit build/test lists, and any other options you might want to specify per-host.

We already support multiple hosts in one run. I don't think there's any difference between supporting one foreign host or supporting several. It's just another spin of the loop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small difference -- for example, we lipo together the foreign compilers for different Darwin slices. But we could just say that if we build any cross-compile any compilers for Darwin hosts, we will lipo them together.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what we do at the moment. Any lipo-able hosts will get merged together unless you tell it to skip that step (again, should really be opt-in instead of opt-out, but only the Darwin hosts are lipo-able anyway)

help="A space-separated list that filters which of the configured "
"targets to build the Swift standard library for, or 'all'.",
type=arguments.type.shell_split, default=["all"])

projects_group = parser.add_argument_group(
title="Options to select projects")
Expand Down Expand Up @@ -1120,6 +1370,10 @@ details of the setups of other systems or automated environments.""")
help="Use the host compiler, not the self-built one to compile the "
"Swift runtime",
action="store_true")
parser.add_argument(
"--build-swift-stdlib-unittest-extra",
help="Build optional StdlibUnittest components",
action="store_true")

run_build_group = parser.add_argument_group(
title="Run build")
Expand Down Expand Up @@ -1480,6 +1734,7 @@ details of the setups of other systems or automated environments.""")
"--build-jobs",
"--common-cmake-options",
"--only-execute",
"--skip-test-optimized",
action=arguments.action.unavailable)

args = migration.parse_args(parser, sys.argv[1:])
Expand Down Expand Up @@ -1541,10 +1796,12 @@ details of the setups of other systems or automated environments.""")
invocation.build_ninja()

# Convert to a build-script-impl invocation.
build_script_impl_args = invocation.convert_to_impl_arguments()
(build_script_impl_env, build_script_impl_args) = \
invocation.convert_to_impl_arguments()

# Execute the underlying build script implementation.
call_without_sleeping([build_script_impl] + build_script_impl_args)
call_without_sleeping([build_script_impl] + build_script_impl_args,
env=build_script_impl_env)

if args.symbols_package:
print('--- Creating symbols package ---')
Expand Down
Loading