-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[SR-237][build-script] Migrate Ninja build to Python #2685
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
Conversation
|
||
# Clean build direcotry if requested. | ||
if args.clean: | ||
shell.rmtree(workspace.build_root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if args.show_sdks:
and if args.clean:
statements seem to be interrupting the flow here. Is there an ordering dependency? If not, could you reorder this code so that first we show SDKs, then clean, create the build directory, then configure ninja and build it?
@swift-ci Please test |
@rintaro Thank you very much for the change and tests! |
@swift-ci Please test |
@rintaro LGTM, just a few small comments. |
d890ff4
to
1c8e687
Compare
…face for build-script.
Migrated impl args: --build-ninja --darwin-deployment-version-{osx,ios,tvos,watchos} Removed impl args: --build-ninja Added impl args: --ninja-bin
1c8e687
to
36898fc
Compare
Fixed. And added missing |
@swift-ci Please test |
What's in this pull request?
More work on [SR-237] Merge
build-script-impl
intobuild-script
.Migrate Ninja build to Python.
Also, some functional changes:
toolchain.cxx
instead ofxcrun --find clang++
to build Ninja.--foundation
is requested as well ascmake_generator == "Ninja"
.--build-ninja
, usebuild-script
detected Ninja path instead of justninja
, because the system installed Ninja can be namedninja-build
.build-script
rm -rf {build_root}
mkdir -p {build_root}
tar -c -z -f ...
Building the standard library for: ...
, after Ninja was built.(Depends Refactored build-script-impl for cross-compiling support #2497)
Please tell me if these changes are unacceptable.
Migrated impl args:
--build-ninja
--darwin-deployment-version-{osx,ios,tvos,watchos}
Removed impl args:
--build-ninja
: Not used inbuild-script-impl
anymoreAdded impl args:
--ninja-bin
: To propagate just builtninja
path tobuild-script-impl
To make it testable, cherry-picked (and enhanced) dry-runnable shell interface from #2241.
Resolved bug number: (Partially SR-237)
Before merging this pull request to apple/swift repository: