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

Share build-script-helper between both tools #26

Merged
merged 8 commits into from Jan 9, 2019

Conversation

Projects
None yet
2 participants
@brentdax
Copy link
Contributor

brentdax commented Dec 22, 2018

This change pulls build-script-helper up to the root of the repository and adds a --package-dir option so it can build either SourceKitStressTester or SwiftEvolve.

Actually using this script requires matching changes in apple/swift#21523; to help ease the transition, this commit leaves a shim script at SourceKitStressTester/Utilities/build-script-helper.sh.

To do:

  • Give this a once-over for style
  • See if I can make the old build-script-helper call through to the new one for compatibility
  • Get this reviewed by Nathan

brentdax added some commits Jan 2, 2019

Copy build-script-helper.py up to project root
I’m temporarily leaving the old SourceKitStressTester build-script-helper in place so I don’t have to coordinate breaking changes to apple/swift’s utils/build-script.

The copied script has not yet been modified to be usable in this location, but committing this version will make it easier to see what changed when reviewing this pull request.
Modify root build-script-helper for both packages
These changes allow an invocation to specify whether it should build SourceKitStressTester or SwiftEvolve. In theory, it should behave virtually identically for SourceKitStressTester, but also work for SwiftEvolve.

This commit also contains matching changes to SwiftEvolve’s package configuration. With those changes, it will build correctly *only* through build-script-helper.

Finally, it includes changes to evolve-swiftCore.sh to build SwiftEvolve through the swift build-script command. This won’t actually work until apple/swift#21523 lands with that support, so evolve-swiftCore.sh is temporarily broken.

@brentdax brentdax force-pushed the brentdax:great-artists-steal branch from a653d97 to 1784337 Jan 2, 2019

@@ -242,5 +244,14 @@ def escape_cmd_arg(arg):
else:
return arg

def get_products(package_dir):
# FIXME: We ought to be able to query SwiftPM for this info.

This comment has been minimized.

@brentdax

brentdax Jan 2, 2019

Contributor

Ankit suggested using dump-package; I plan to do that in the future.

@brentdax brentdax force-pushed the brentdax:great-artists-steal branch from 884607f to d8fa16c Jan 3, 2019

Replace old build-script-helper with shim script
The modified version locates and calls through to the new script rather than duplicating all of its logic.

@brentdax brentdax force-pushed the brentdax:great-artists-steal branch from d8fa16c to 20104c8 Jan 3, 2019

brentdax added some commits Jan 4, 2019

Fix accidentally hardcoded build dir
Code previously used to calculate a default value was changed to being always run without updating it to use the non-default value. Oops.
Build less in evolve-swiftCore driver
Now that we’re properly integrated, we don’t need to explicitly build SwiftPM anymore.

@brentdax brentdax requested a review from nathawes Jan 4, 2019

@brentdax brentdax changed the title [WIP] Share build-script-helper between both tools Share build-script-helper between both tools Jan 4, 2019

@brentdax

This comment has been minimized.

Copy link
Contributor

brentdax commented Jan 8, 2019

Ran a "build toolchain" on the apple/swift PR to test this, and got a failure that seems to be on this side of the fence. https://ci.swift.org/job/swift-PR-toolchain-osx/152/console

Show resolved Hide resolved build-script-helper.py Outdated
Lift several install lines out of the product loop
The rpaths_to_delete line is the probable cause of a failure; the others are just unnecessary to run repeatedly.
@brentdax

This comment has been minimized.

Copy link
Contributor

brentdax commented Jan 9, 2019

With that change, CI can now build toolchains. I downloaded it and verified that both sk-stress-test and swift-evolve are present and can at least print their --help text.

@nathawes Any other comments on this, or do you think it's good to merge?

@nathawes
Copy link
Contributor

nathawes left a comment

It looks good to me – thanks!

@brentdax brentdax merged commit 368cddc into apple:master Jan 9, 2019

@brentdax brentdax deleted the brentdax:great-artists-steal branch Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment