Updated the Windows build documentation: #17309
Conversation
|
cc @compnerd |
|
@swift-ci Please smoke test |
|
@dplanitzer you need to be a committer to use swift-ci. See: https://github.com/apple/swift/blob/master/docs/ContinuousIntegration.md#swift-ci I got you though. ; ). |
|
@swift-ci smoke test |
|
@gottesmm ah I didn't read the fine print :) Thanks! |
docs/WindowsBuild.md
Outdated
| @@ -25,6 +25,7 @@ Windows. | |||
| - Using the latest Visual Studio version is recommended (tested with Visual | |||
| Studio 2017 - Version 15.5.5). Swift may fail to build with older C++ | |||
| compilers. | |||
| - Note that the release version of Swift on Windows may crash when you try to compile a Swift program. See bug report [SR-7867](https://bugs.swift.org/browse/SR-7867). | |||
There was a problem hiding this comment.
Formatting nit: would you mind wrapping the text you add to 80 characters?
|
@xwu wrapped text to 80 characters and added an additional bullet point to the install section to document that the Swift/bin directory has to be added to the Path environment variable too. |
docs/WindowsBuild.md
Outdated
| 1. Clone `apple/swift-cmark` into a folder named `swift-cmark` | ||
| 1. Clone `apple/swift-clang` into a folder named `swift-clang` | ||
| 1. Clone `apple/swift-llvm` into a folder named `swift-llvm` | ||
| 1. Clone `apple/swift-compiler-rt` into a folder named `swift-compiler-rt` |
There was a problem hiding this comment.
Why are you changing the names here? This layout is similar to what is used on Linux and macOS.
There was a problem hiding this comment.
Because these are the folder names that we get when we do a git clone with the repository URL as published by GitHub:
$ git clone https://github.com/apple/swift-cmark.git
Cloning into 'swift-cmark'...
remote: Counting objects: 9101, done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 9101 (delta 13), reused 1 (delta 0), pack-reused 9087
Receiving objects: 100% (9101/9101), 2.79 MiB | 3.71 MiB/s, done.
Resolving deltas: 100% (6524/6524), done.
$ ls
swift-cmarkThere was a problem hiding this comment.
Sure, but the expectation is that the repositories will be renamed. See https://github.com/apple/swift/blob/master/utils/update-checkout-config.json
docs/WindowsBuild.md
Outdated
| 1. Clone `apple/swift` into a folder named `swift` | ||
| - Currently, other repositories in the Swift project have not been tested and | ||
| may not be supported. | ||
|
|
||
| ### 3. Build ICU | ||
| 1. Download and extract the [ICU source | ||
| code](http://site.icu-project.org/download) to a folder named `icu` in the same | ||
| directory as the other Swift project repositories. | ||
| directory as the other Swift project repositories (tested with ICU version 55.1). |
There was a problem hiding this comment.
I'll update the documentation accordingly.
docs/WindowsBuild.md
Outdated
| @@ -152,8 +160,13 @@ cmake -G "Ninja" "%swift_source_dir%/swift"^ | |||
| -DSWIFT_INCLUDE_TESTS=FALSE^ | |||
| -DCMAKE_C_COMPILER="%llvm_bin_dir%/clang-cl.exe"^ | |||
| -DCMAKE_CXX_COMPILER="%llvm_bin_dir%/clang-cl.exe"^ | |||
| -DCMAKE_C_FLAGS="-fms-compatibility-version=19.00 /Z7"^ | |||
| -DCMAKE_CXX_FLAGS="-fms-compatibility-version=19.00 -Z7" ^ | |||
| -DCMAKE_C_FLAGS="-fms-compatibility-version=19.00 /Z7 -Wno-c11-extensions -Wno-language-extension-token -Wno-reserved-id-macro -Wno-disabled-macro-expansion"^ | |||
There was a problem hiding this comment.
I don't think that the flags are needed any longer. The warnings that are superfluous we should be disabling a different way I think.
There was a problem hiding this comment.
We still get these warnings with the TOT code:
C:\Users\dplan\Documents\Developer\swift-repos\swift\stdlib\public\runtime\ImageInspectionCOFF.cpp(106,9): warning: 'auto' type specifier is incompatible with C++98 [-Wc++98-compat]
const auto &protocols_section = sections->swift5_protocols;
^~~~
C:\Users\dplan\Documents\Developer\swift-repos\swift\stdlib\public\runtime\SwiftDtoa.cpp(319,12): warning: use of old-style cast [-Wold-style-cast]
= ((uint64_t)1 << significandBitCount) - 1;
^ ~
How else would you turn those expected warnings off? I'm not too concerned with whether this part looks good or not. My main concern is to increase the chance that folks who are interested in working on the Windows port will see the problematic warnings (especially the ones about shortening 64bit values to 32bit values without an explicit cast as a conformation that, yes, someone checked that this is actually correct code).
Eventually we'll want to use the build script on Windows anyway and there shouldn't be a need anymore to document the low-level build steps.
There was a problem hiding this comment.
I think that a couple of these options are fine, but I would rather that we do it through CMake so that they work even when cross-compiling.
docs/WindowsBuild.md
Outdated
| -DCMAKE_EXE_LINKER_FLAGS:STRING="/machine:x64 /INCREMENTAL:NO"^ | ||
| -DCMAKE_MODULE_LINKER_FLAGS="/machine:x64 /INCREMENTAL:NO"^ | ||
| -DCMAKE_SHARED_LINKER_FLAGS="/machine:x64 /INCREMENTAL:NO"^ | ||
| -DCMAKE_STATIC_LINKER_FLAGS="/machine:x64 /INCREMENTAL:NO"^ |
There was a problem hiding this comment.
The /machine flag shouldn't be needed. That is addressed with using the right Developer Command Prompt environment.
There was a problem hiding this comment.
I can remove this if you want me to. I simply preserved what cmake generated by default.
|
Updated the documentation with information that ICU 59.1 is also a tested config. |
- made the build type consistent across all subprojects - point out that the release build may crash - link the Swift compiler and runtime libraries with incremental linking turned off - suppress expected warnings - added missing build parameters to the lldb cmake invocation line - added simple installation instructions
|
@compnerd incorporated your comments |
|
@swift-ci please smoke test |
|
I think that this is ready to merge now, right? |
|
Yes, looks like this is approved and ready to merge. |
Most importantly, changed the build steps for the Swift compiler and runtime libraries such that incremental linking is turned off which is necessary to ensure that a program compiled with the Swift compiler will actually work. See bug SR-7991.