Skip to content

Get SwiftSyntax ready for building in a unified build with other projects #167

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

Merged
merged 1 commit into from
Nov 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ let swiftSyntaxTarget: PackageDescription.Target

/// If we are in a controlled CI environment, we can use internal compiler flags
/// to speed up the build or improve it.
if getenv("SWIFT_SYNTAX_CI_ENVIRONMENT") != nil {
if getenv("SWIFT_BUILD_SCRIPT_ENVIRONMENT") != nil {
let groupFile = URL(fileURLWithPath: #file)
.deletingLastPathComponent()
.appendingPathComponent("utils")
Expand All @@ -48,9 +48,9 @@ package.targets.append(swiftSyntaxTarget)

let libraryType: Product.Library.LibraryType

/// When we're in a CI environment, we want to build a dylib instead of a static
/// library since we install the dylib into the toolchain.
if getenv("SWIFT_SYNTAX_CI_ENVIRONMENT") != nil {
/// When we're in a build-script environment, we want to build a dylib instead
/// of a static library since we install the dylib into the toolchain.
if getenv("SWIFT_BUILD_SCRIPT_ENVIRONMENT") != nil {
libraryType = .dynamic
} else {
libraryType = .static
Expand Down
35 changes: 28 additions & 7 deletions build-script.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ def generate_gyb_files(verbose, add_source_locations, destination=None):
def get_installed_dylib_name():
return 'libSwiftSyntax.dylib'

def get_swiftpm_invocation(toolchain, action, build_dir, release):
def get_swiftpm_invocation(toolchain, action, build_dir, multiroot_data_file,
release):
swift_exec = os.path.join(toolchain, 'usr', 'bin', 'swift')

swiftpm_call = [swift_exec, action]
Expand All @@ -167,15 +168,18 @@ def get_swiftpm_invocation(toolchain, action, build_dir, release):
swiftpm_call.extend(['--configuration', 'release'])
if build_dir:
swiftpm_call.extend(['--build-path', build_dir])
if multiroot_data_file:
swiftpm_call.extend(['--multiroot-data-file', multiroot_data_file])

return swiftpm_call

class Builder(object):
def __init__(self, toolchain, build_dir, release, verbose,
disable_sandbox=False):
def __init__(self, toolchain, build_dir, multiroot_data_file, release,
verbose, disable_sandbox=False):
self.swiftpm_call = get_swiftpm_invocation(toolchain=toolchain,
action='build',
build_dir=build_dir,
multiroot_data_file=multiroot_data_file,
release=release)
if disable_sandbox:
self.swiftpm_call.append('--disable-sandbox')
Expand All @@ -189,7 +193,9 @@ def build(self, product_name, module_group_path=''):
command.extend(['--product', product_name])

env = dict(os.environ)
env['SWIFT_SYNTAX_CI_ENVIRONMENT'] = '1'
env['SWIFT_BUILD_SCRIPT_ENVIRONMENT'] = '1'
# Tell other projects in the unified build to use local dependencies
env['SWIFTCI_USE_LOCAL_DEPS'] = '1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need two environment variables instead of only one?

Copy link
Member Author

Choose a reason for hiding this comment

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

My idea was that someone would still be able to build a non-build-script build (i.e. static libraries, no enforce-exclusivity=unchecked) while still using the already checked out sources.

Similarly when building the stress tester you might want to create a CI-style build without manually checking out SwiftSyntax etc. You could do that by specifying --no-local-dependencies on the stress tester's build script.

It just feels like these are two different things that we should not mush together into a single flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, sounds good, but then should SWIFTCI_USE_LOCAL_DEPS have a more general name, like SWIFT_BUILD_USE_LOCAL_DEPS or something, and not seem like it should be only set by the CI system?

Copy link
Member Author

Choose a reason for hiding this comment

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

The name needs to match the one used in the Swift package manager here to avoid it from pulling in remote dependencies. I could go and change it there first.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not important, the exact spelling doesn't matter much

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. I’ll just leave it as is for now then.

check_call(command, env=env, verbose=self.verbose)


Expand All @@ -212,7 +218,8 @@ def verify_generated_files(verbose):
check_call(command)


def run_tests(toolchain, build_dir, release, filecheck_exec, verbose):
def run_tests(toolchain, build_dir, multiroot_data_file, release,
filecheck_exec, verbose):
print('** Running SwiftSyntax Tests **')

lit_success = run_lit_tests(toolchain=toolchain,
Expand All @@ -225,6 +232,7 @@ def run_tests(toolchain, build_dir, release, filecheck_exec, verbose):

xctest_success = run_xctests(toolchain=toolchain,
build_dir=build_dir,
multiroot_data_file=multiroot_data_file,
release=release,
verbose=verbose)
if not xctest_success:
Expand Down Expand Up @@ -259,6 +267,7 @@ def find_lit_test_helper_exec(toolchain, build_dir, release):
swiftpm_call = get_swiftpm_invocation(toolchain=toolchain,
action='build',
build_dir=build_dir,
multiroot_data_file=None,
release=release)
swiftpm_call.extend(['--product', 'lit-test-helper'])
swiftpm_call.extend(['--show-bin-path'])
Expand Down Expand Up @@ -298,18 +307,23 @@ def run_lit_tests(toolchain, build_dir, release, filecheck_exec, verbose):

## XCTest based tests

def run_xctests(toolchain, build_dir, release, verbose):
def run_xctests(toolchain, build_dir, multiroot_data_file, release, verbose):
print('** Running XCTests **')
swiftpm_call = get_swiftpm_invocation(toolchain=toolchain,
action='test',
build_dir=build_dir,
multiroot_data_file=multiroot_data_file,
release=release)

if verbose:
swiftpm_call.extend(['--verbose'])

swiftpm_call.extend(['--test-product', 'SwiftSyntaxPackageTests'])

env = dict(os.environ)
env['SWIFT_SYNTAX_CI_ENVIRONMENT'] = '1'
env['SWIFT_BUILD_SCRIPT_ENVIRONMENT'] = '1'
# Tell other projects in the unified build to use local dependencies
env['SWIFTCI_USE_LOCAL_DEPS'] = '1'
return call(swiftpm_call, env=env, verbose=verbose) == 0

def delete_rpath(rpath, binary):
Expand Down Expand Up @@ -433,6 +447,11 @@ def main():
help='The script only generates swift files from gyb '
'and skips the rest of the build')

build_group.add_argument('--multiroot-data-file',
help='Path to an Xcode workspace to create a '
'unified build of SwiftSyntax with other '
'projects.')

testing_group = parser.add_argument_group('Testing')
testing_group.add_argument('-t', '--test', action='store_true',
help='Run tests')
Expand Down Expand Up @@ -494,6 +513,7 @@ def main():
try:
builder = Builder(toolchain=args.toolchain,
build_dir=args.build_dir,
multiroot_data_file=args.multiroot_data_file,
release=args.release,
verbose=args.verbose,
disable_sandbox=args.disable_sandbox)
Expand All @@ -512,6 +532,7 @@ def main():
try:
success = run_tests(toolchain=args.toolchain,
build_dir=realpath(args.build_dir),
multiroot_data_file=args.multiroot_data_file,
release=args.release,
filecheck_exec=realpath(args.filecheck_exec),
verbose=args.verbose)
Expand Down