Skip to content

[build-script-impl] Add support for performing isolated actions. #2844

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 1 commit into from
Jun 3, 2016
Merged

[build-script-impl] Add support for performing isolated actions. #2844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions utils/build-script
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,7 @@ details of the setups of other systems or automated environments.""")
# Explicitly unavailable options here.
"--build-jobs",
"--common-cmake-options",
"--only-execute",
action=arguments.action.unavailable)

args = migration.parse_args(parser, sys.argv[1:])
Expand Down
106 changes: 96 additions & 10 deletions utils/build-script-impl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ KNOWN_SETTINGS=(
common-cmake-options "" "CMake options used for all targets, including LLVM/Clang"
# TODO: Remove this some time later.
user-config-args "" "**Renamed to --extra-cmake-options**: User-supplied arguments to cmake when used to do configuration."
only-execute "all" "Only execute the named action (see implementation)"
)

# Centalized access point for traced command invocation.
Expand Down Expand Up @@ -308,6 +309,51 @@ function float_min() {
fi
}

# Support for performing isolated actions.
#
# This is part of refactoring more work to be done or controllable via
# `build-script` itself. For additional information, see:
# https://bugs.swift.org/browse/SR-237
#
# To use this functionality, the script is invoked with:
# ONLY_EXECUTE=<action name>
# where <action name> is one of:
# all -- execute all actions
# ${host}-${product}-build -- the build of the product
# ${host}-${product}-test -- the test of the product
# ${host}-${product}-install -- the install of the product
# ${host}-package -- the package construction and test
# merged-hosts-lipo -- the lipo step, if used
# and if used, only the one individual action will be performed.
#
# If not set, the default is `all`.

# should_execute_action(name) -> 1 or nil
#
# Check if the named action should be run in the given script invocation.
function should_execute_action() {
local name="$1"

if [[ "${ONLY_EXECUTE}" = "all" ]] ||
[[ "${ONLY_EXECUTE}" = "${name}" ]]; then
echo 1
fi
}

# should_execute_host_actions_for_phase(host, phase-name) -> 1 or nil
#
# Check if the there are any actions to execute for this host and phase (i.e.,
# "build", "test", or "install")
function should_execute_host_actions_for_phase() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for moving this into a function! The old logic seemed simple, but it turned out to be somehow confusing for me. With a function it is much more clear what is happening.

local host="$1"
local phase_name="$2"

if [[ "${ONLY_EXECUTE}" = "all" ]] ||
[[ "${ONLY_EXECUTE}" == ${host}-*-${phase_name} ]]; then
echo 1
fi
}

function num_llvm_parallel_lto_link_jobs() {
case "$(uname -s -m)" in
Darwin\ x86_64)
Expand Down Expand Up @@ -1512,19 +1558,26 @@ function set_swiftpm_bootstrap_command() {


for host in "${ALL_HOSTS[@]}"; do
# Skip this pass when the only action to execute can't match.
if ! [[ $(should_execute_host_actions_for_phase ${host} build) ]]; then
continue
fi

calculate_targets_for_host $host

set_build_options_for_host $host

echo "Building the standard library for: ${SWIFT_STDLIB_TARGETS[@]}"
if [[ "${SWIFT_TEST_TARGETS[@]}" ]] && ! [[ "${SKIP_TEST_SWIFT}" ]]; then
echo "Running Swift tests for: ${SWIFT_TEST_TARGETS[@]}"
fi
if ! [[ "${SKIP_TEST_BENCHMARKS}" ]] &&
[[ "${SWIFT_RUN_BENCHMARK_TARGETS[@]}" ]] &&
! [[ "${SKIP_TEST_BENCHMARK}" ]]; then
echo "Running Swift benchmarks for: ${SWIFT_RUN_BENCHMARK_TARGETS[@]}"
# Don't echo anything if only executing an individual action.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not? This information is very useful in build logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but if left here it would be printed multiple times. My plan was to make the build-script print this information when it switched over to --only-execute.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid the build-script does not have this information yet, it does not know the specific targets that will be executed. (It is important to know whether we will be doing short vs. validation vs. long tests for example, or which platforms the tests will be run for.) Did you plan to move this computation over as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to eventually, so it seems like we might as well do that as part of switching the top-level loop over than come up with another mechanism to get this printed at the right time (I have a strong preference for not have unnecessary extra output).

if [[ "${ONLY_EXECUTE}" = "all" ]]; then
echo "Building the standard library for: ${SWIFT_STDLIB_TARGETS[@]}"
if [[ "${SWIFT_TEST_TARGETS[@]}" ]] && ! [[ "${SKIP_TEST_SWIFT}" ]]; then
echo "Running Swift tests for: ${SWIFT_TEST_TARGETS[@]}"
fi
if ! [[ "${SKIP_TEST_BENCHMARKS}" ]] &&
[[ "${SWIFT_RUN_BENCHMARK_TARGETS[@]}" ]] &&
! [[ "${SKIP_TEST_BENCHMARK}" ]]; then
echo "Running Swift benchmarks for: ${SWIFT_RUN_BENCHMARK_TARGETS[@]}"
fi
fi

common_cmake_options_host=("${COMMON_CMAKE_OPTIONS[@]}")
Expand Down Expand Up @@ -1644,6 +1697,11 @@ for host in "${ALL_HOSTS[@]}"; do
)

for product in "${PRODUCTS[@]}"; do
# Check if we should perform this action.
if ! [[ $(should_execute_action "${host}-${product}-build") ]]; then
continue
fi

unset skip_build
source_dir_var="$(toupper ${product})_SOURCE_DIR"
source_dir=${!source_dir_var}
Expand Down Expand Up @@ -2198,12 +2256,21 @@ tests_busted ()
}

for host in "${ALL_HOSTS[@]}"; do
# Skip this pass when the only action to execute can't match.
if ! [[ $(should_execute_host_actions_for_phase ${host} test) ]]; then
continue
fi

# Calculate test targets
calculate_targets_for_host $host

# Run the tests for each product
for product in "${PRODUCTS[@]}"; do
# Check if we should perform this action.
if ! [[ $(should_execute_action "${host}-${product}-test") ]]; then
continue
fi

case ${product} in
cmark)
if [[ "${SKIP_TEST_CMARK}" ]]; then
Expand Down Expand Up @@ -2410,6 +2477,10 @@ done
LIPO_SRC_DIRS=()

for host in "${ALL_HOSTS[@]}"; do
# Skip this pass when the only action to execute can't match.
if ! [[ $(should_execute_host_actions_for_phase ${host} install) ]]; then
continue
fi

# Calculate the directory to install products in to.
host_install_destdir=$(get_host_install_destdir ${host})
Expand All @@ -2423,6 +2494,10 @@ for host in "${ALL_HOSTS[@]}"; do
set_build_options_for_host $host

for product in "${PRODUCTS[@]}"; do
# Check if we should perform this action.
if ! [[ $(should_execute_action "${host}-${product}-install") ]]; then
continue
fi

INSTALL_TARGETS="install"

Expand Down Expand Up @@ -2729,6 +2804,11 @@ function build_and_test_installable_package() {
# Build and test packages.
for host in "${ALL_HOSTS[@]}"; do

# Check if we should perform this action.
if ! [[ $(should_execute_action "${host}-package") ]]; then
continue
fi

if [[ $(should_include_host_in_lipo ${host}) ]]; then
continue
fi
Expand All @@ -2738,11 +2818,17 @@ done

# Lipo those products which require it, optionally build and test an installable package.
if [[ ${#LIPO_SRC_DIRS[@]} -gt 0 ]]; then
echo "--- Merging and running lipo ---"

# This is from multiple hosts; Which host should we say it is?
# Let's call it 'merged-hosts' so that we can identify it.
mergedHost="merged-hosts"

# Check if we should perform this action.
if ! [[ $(should_execute_action "${mergedHost}-lipo") ]]; then
continue
fi

echo "--- Merging and running lipo ---"

call "${SWIFT_SOURCE_DIR}"/utils/recursive-lipo --lipo=$(xcrun_find_tool lipo) --copy-subdirs="$(get_host_install_prefix ${host})lib/swift $(get_host_install_prefix ${host})lib/swift_static" --destination="$(get_host_install_destdir ${mergedHost})" ${LIPO_SRC_DIRS[@]}

# Build and test the lipo-ed package.
Expand Down