Skip to content

Added more details to "Building Swift on Windows" #28496

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 7, 2020
Merged

Added more details to "Building Swift on Windows" #28496

merged 4 commits into from
Jan 7, 2020

Conversation

yostane
Copy link
Contributor

@yostane yostane commented Nov 27, 2019 •
edited
Loading

Hi. I have added more details on building Swift on Windows. I added further details in three aspects:

  • I gave the names that correspond to the Visual Studio Component IDs
  • I added a link for each Library
  • I added an image that shows how to launch the Visual Studio Command Prompt

I hope that these additions will make the build process more straightforward :).

I also took the opportunity to clean up the markdown file.
Thanks for the review,

@theblixguy theblixguy requested a review from compnerd November 28, 2019 00:12
@yostane yostane marked this pull request as ready for review November 28, 2019 22:56
@compnerd
Copy link
Member

compnerd commented Dec 1, 2019

The image doesn't render? I'm not sure that we really want images as part of the directions though - this is something we should be doing to simplify the build, not adding images.

Why the link to the french translation?

The names for the components can change, I think it probably makes more sense to provide instructions to automate the installation of the features through an unattended installation invocation.

@yostane
Copy link
Contributor Author

yostane commented Dec 2, 2019 •
edited
Loading

Thanks for the interesting reviews. Here are my replies.

The image doesn't render? I'm not sure that we really want images as part of the directions though - this is something we should be doing to simplify the build, not adding images.

I removed the image. I just wanted to insist on launching the Visual Studio command line and not the regular one.

Why the link to the french translation?

Fixed the link. It allows to find the component name from its ID. It is useful when we select components from Visual Studio installer IU.

The names for the components can change, I think it probably makes more sense to provide instructions to automate the installation of the features through an unattended installation invocation.

I removed the names of the components and just kept the link.

About the instruction, I agree :). Thanks to your suggestion, I have found that the visual studio installer can install components from the command line. It would be interesting to investigate this. For example, we can provide commands that install the necessary packages for VS2019.
https://docs.microsoft.com/visualstudio/install/use-command-line-parameters-to-install-visual-studio?view=vs-2019

If possible, I suggest to work on the installer cli in another PR.

compnerd
compnerd reviewed Dec 2, 2019
View reviewed changes
docs/WindowsBuild.md Outdated
@@ -71,7 +91,9 @@ Warning: Creating the above links usually requires administrator privileges. The

```cmd
md "S:\b\toolchain"
cmake -B "S:\b\toolchain" -G Ninja -S S:\toolchain\llvm -C S:\windows-swift\cmake\caches\Windows-x86_64.cmake -C S:\windows-swift\cmake\caches\org.compnerd.dt.cmake -DLLVM_ENABLE_ASSERTIONS=YES -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;cmark;swift;lldb;lld" -DLLVM_EXTERNAL_PROJECTS="cmark;swift" -DSWIFT_PATH_TO_LIBDISPATCH_SOURCE=S:\toolchain\swift-corelibs-libdispatch -DLLVM_ENABLE_PDB=YES -DLLDB_DISABLE_PYTHON=YES -DSWIFT_WINDOWS_x86_64_ICU_UC_INCLUDE="S:/Library/icu-64/usr/include" -DSWIFT_WINDOWS_x86_64_ICU_UC="S:/Library/icu-64/usr/lib/icuuc64.lib" -DSWIFT_WINDOWS_x86_64_ICU_I18N_INCLUDE="S:/Library/icu-64/usr/include" -DSWIFT_WINDOWS_x86_64_ICU_I18N="S:/Library/icu-64/usr/lib/icuin64.lib" -DCMAKE_INSTALL_PREFIX="C:\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr" -DPYTHON_EXECUTABLE=C:\Python27\python.exe -DSWIFT_BUILD_DYNAMIC_STDLIB=YES -DSWIFT_BUILD_DYNAMIC_SDK_OVERLAY=YES
set PYTHON_EXE=(your python.exe full path)
set INSTALL_PREFIX=C:\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain\usr
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the value that is gained from using the extra variable here.

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 added the extra variables to reduce the size of the commands. It also allows to reuse in other commands where the python path is needed.

For example, my python.exe has a quite log path ...\AppData\Local\Programs\Python\Python38\python.exe

Copy link
Member

Choose a reason for hiding this comment

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

The paths are going to be long, the majority of the commands are just paths, they are not intrinsically long. The reason for the commands being this way is to aid in copy/paste execution and recall in cmd which doesn't have a lot of the nice features of Unix. Id really prefer not relying on extra variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. I rolled back my modifications on this part

docs/WindowsBuild.md
- [SQLite](https://dev.azure.com/compnerd/windows-swift/_build?definitionId=12&_a=summary)
- [curl](https://dev.azure.com/compnerd/windows-swift/_build?definitionId=11&_a=summary)
- [libxml2](https://dev.azure.com/compnerd/windows-swift/_build?definitionId=10&_a=summary)
- [zlib](https://dev.azure.com/compnerd/windows-swift/_build?definitionId=16&_a=summary)
Copy link
Member

Choose a reason for hiding this comment

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

Good idea with the links. However, I think that we can actually do this much better now. I've been playing around with more automation and something like this works really well:

pip install azure
C:\Python27\python.exe windows-swift\utilities\windows-swift.py --build 10 --build 11 --build 12 --build 16 --download-latest-artifacts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :)
I agree that the command looks great. We can keep the link for information purpose. What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

Sure

@yostane
Copy link
Contributor Author

yostane commented Jan 7, 2020 •
edited
Loading

Hello @compnerd and happy new year.

Can the PR be accepted as it is right now ?

Thanks,

@compnerd
Copy link
Member

compnerd commented Jan 7, 2020

@swift-ci please smoke test

@compnerd compnerd merged commit 8c2854f into swiftlang:master Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yostane @compnerd