Skip to content

[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

Merged
merged 2 commits into from
May 26, 2016

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented May 25, 2016

What's in this pull request?

More work on [SR-237] Merge build-script-impl into build-script.

Migrate Ninja build to Python.
Also, some functional changes:

  • In OS X, use user configurable toolchain.cxx instead of xcrun --find clang++ to build Ninja.
  • When the system installed Ninja is not found, try to build Ninja if --foundation is requested as well as cmake_generator == "Ninja".
  • To build Foundation, even without --build-ninja, use build-script detected Ninja path instead of just ninja, because the system installed Ninja can be named ninja-build.
  • Added debug trace output for commands used in build-script
    • Clean build directory rm -rf {build_root}
    • Create build directory mkdir -p {build_root}
    • Archive symbols package: tar -c -z -f ...
  • Possible regression:

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 in build-script-impl anymore

Added impl args:
--ninja-bin: To propagate just built ninja path to build-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:

  • Test pull request on Swift continuous integration.


# Clean build direcotry if requested.
if args.clean:
shell.rmtree(workspace.build_root)
Copy link
Contributor

@gribozavr gribozavr May 25, 2016

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?

@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr
Copy link
Contributor

@rintaro Thank you very much for the change and tests!

@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr
Copy link
Contributor

@rintaro LGTM, just a few small comments.

@rintaro rintaro force-pushed the build-script-build-ninja branch from d890ff4 to 1c8e687 Compare May 26, 2016 01:28
rintaro added 2 commits May 26, 2016 10:29
Migrated impl args:
--build-ninja
--darwin-deployment-version-{osx,ios,tvos,watchos}

Removed impl args:
--build-ninja

Added impl args:
--ninja-bin
@rintaro rintaro force-pushed the build-script-build-ninja branch from 1c8e687 to 36898fc Compare May 26, 2016 01:30
@rintaro
Copy link
Member Author

rintaro commented May 26, 2016

Fixed. And added missing xcrun.sdk_path test case.
Omitting stderr=subprocess.PIPE mess up python -m unittest discover output, but it's harmless.

@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr gribozavr merged commit ad6de9b into swiftlang:master May 26, 2016
@rintaro rintaro deleted the build-script-build-ninja branch May 26, 2016 06:02
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.

2 participants