Skip to content

Move build-script apply default arguments into separate module #11882

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

Conversation

Rostepher
Copy link
Contributor

@Rostepher Rostepher commented Sep 12, 2017

Purpose

This PR moves the apply_default_arguments logic out of the main build-script code into the driver_arguments module in order to better stage for #11872 and eventually #11880. The diff will show that the move was not strictly a copy-paste, the chunk for determining if ninja ought be built requires the toolchain information which only exists in the BuildScriptInvocation class. I've opted instead to single out that single case and keep it where it was, everything else was moved verbatim.

rdar://problem/34336890

@Rostepher Rostepher changed the title Move build script apply defaults Move build-script apply default arguments into separate module Sep 12, 2017
…nts module in preparation for the larger argument parsing refactor.
@Rostepher Rostepher force-pushed the move-build-script-apply-defaults branch from fdcd085 to 619855b Compare September 12, 2017 23:40
@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher
Copy link
Contributor Author

As soon as the builds for this branch pass I'll merge it, then onwards to #11872!

@Rostepher Rostepher merged commit d4d35ac into swiftlang:master Sep 14, 2017
@Rostepher Rostepher deleted the move-build-script-apply-defaults branch September 14, 2017 00:09
@davezarzycki
Copy link
Contributor

This broke the build-script on my machine. It can no longer find ninja in /usr/local/bin. What do you need to help debug this?

@Rostepher
Copy link
Contributor Author

@davezarzycki I've made a PR to address the issue #11919. I'll have it merged tonight or in the morning, Sorry for the inconvenience.

@tishansteve
Copy link

tishansteve commented Sep 14, 2017 via email

@Rostepher
Copy link
Contributor Author

@tishansteve You can change your email preferences in your Github account settings. I can't do anything about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants