Skip to content

[READY FOR REVIEW][DNM] Refresh WindowsBuild.md #41560

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

stevapple
Copy link
Contributor

@stevapple stevapple commented Feb 25, 2022

Polish Windows build guide from head to tail:

  1. Update recommended version to VS2019 and Python 3.9;
  2. Use update-checkout instead of manual checkout;
  3. Split the artifacts into SDK and toolchain;
  4. No longer suggests -DLLVM_ENABLE_PDB=YES with Swift compiler;
  5. Use vcpkg to pull in dependencies;
  6. Strip unnecessary options;
  7. Add SourceKit-LSP to build.

Future direction will be adding not-yet-supported build steps for anyone who would like to fix.

Resolves SR-NNNN.

@stevapple
Copy link
Contributor Author

Something to check with @compnerd
Is ATL really used in Swift? If the answer is yes, which part of it?

@stevapple stevapple force-pushed the windows-build branch 3 times, most recently from de227ab to f48eefe Compare February 28, 2022 04:24
@stevapple stevapple changed the title [WindowsBuild] Polish the guide and UX [DO NOT MERGE] Refresh WindowsBuild.md Feb 28, 2022
@stevapple stevapple marked this pull request as ready for review February 28, 2022 11:09
@stevapple
Copy link
Contributor Author

stevapple commented Feb 28, 2022

I believe this guide is ready for review, but here are still some related PRs that had better go first.

@stevapple stevapple force-pushed the windows-build branch 4 times, most recently from 96ba4bb to d8b6901 Compare March 5, 2022 12:15
@stevapple stevapple force-pushed the windows-build branch 2 times, most recently from 7ad643d to aab1aa6 Compare March 14, 2022 06:46
@stevapple
Copy link
Contributor Author

For anyone who may be concerned: This PR is fully ready for review now. I have validated all the build steps with a fresh build locally, including the functionality of built toolchain.

Before it can get merged, we’d better address the issues listed in #41560 (comment). However, the guide itself should be complete and qualified.

@stevapple stevapple changed the title [DO NOT MERGE] Refresh WindowsBuild.md [READY FOR REVIEW][DNM] Refresh WindowsBuild.md Mar 14, 2022
@stevapple stevapple force-pushed the windows-build branch 5 times, most recently from 0dd5a2e to 69586da Compare March 14, 2022 08:27
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

I've not gone through everything, but please change the existing things, I'll look through the rest after those changes are made.

5. Click *Install*

## Enable Developer Mode
### Set up `vcpkg`
Copy link
Member

Choose a reason for hiding this comment

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

Please add instructions for building from source rather than vcpkg. I do not wish to have this be the recommended path. Having vcpkg as an option in an appendix is okay though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vcpkg doesn’t have prebuilt binaries. It will build everything from source.


The instructions assume that the dependencies are in `S:/Library`. The directory
structure should resemble:
You'll be able to clone and check out the rest of Swift source repositories with `update-checkout` tool:
Copy link
Member

Choose a reason for hiding this comment

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

Add the commands for cloning the sources for dependencies please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer vcpkg over custom builds. We’re not telling a story of how to build a standard toolchain. This one is just for debugging (as is mentioned in the note), so the easiest & most efficient way should be recommended.

path S:\b\3\bin;%PATH%
cmake -B S:\b\5 ^
-D CMAKE_BUILD_TYPE=RelWithDebInfo ^
-D CMAKE_INSTALL_PREFIX=S:\b\sdk\usr ^
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t think we need the path. Why? For TestFoundation support, see swiftlang/swift-corelibs-foundation#3146


```cmd
cmake -B S:\b\4 ^
-D CMAKE_BUILD_TYPE=RelWithDebInfo ^
-D CMAKE_INSTALL_PREFIX=C:\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr ^
-D CMAKE_INSTALL_PREFIX=S:\b\sdk\usr ^
Copy link
Member

Choose a reason for hiding this comment

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

Please restore the original path.

Copy link
Contributor Author

@stevapple stevapple Mar 14, 2022

Choose a reason for hiding this comment

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

Again — this guide is for debugging purpose, fixing to standard path is unnecessary, and will conflict with pre-installed Swift toolchains. Having it installed next to the source will empower us to test the toolchain standalone.

-D LIT_COMMAND=S:\llvm-project\llvm\utils\lit\lit.py ^
-D dispatch_DIR=S:\b\3\cmake\modules ^

-D CMAKE_TOOLCHAIN_FILE=S:\vcpkg\scripts\buildsystems\vcpkg.cmake ^
Copy link
Member

Choose a reason for hiding this comment

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

Please drop this and the same througout.

@stevapple
Copy link
Contributor Author

Addressed some of the feedbacks reasonable from my side.

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