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

Update docs for Bash on Ubuntu on Windows #5671

Merged
merged 4 commits into from Nov 13, 2016

Conversation

Projects
None yet
6 participants
@hughbe
Copy link
Collaborator

hughbe commented Nov 7, 2016

With these modifications, Swift can be compiled and run on Windows, via Bash on Windows, also known as WSL

See full instructions here

@CodaFi

This comment has been minimized.

Copy link
Collaborator

CodaFi commented Nov 7, 2016

Fantastic, but is there a way to untie this patch from a specific CMake release? I worry about keeping the README in sync with future CMake requirement bumps.

@hughbe hughbe force-pushed the hughbe:wsl-build-instructions branch 2 times, most recently Nov 7, 2016

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Nov 7, 2016

I feel like this is overkill. Swift currently officially supports development three platforms: macOS, Ubuntu 16.04, and Ubuntu 16.10 (latter two recently updated). The build instructions will also work for many other POSIXy systems. Ubuntu 14.04 is a past supported release, so calling out the Clang requirement there made sense; now, it's probably better to just highlight the required version of Clang and CMake generically rather than referencing any particular build or host platform.

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 7, 2016

@CodaFi Unfortunately, WSL is stuck on Ubuntu 14.0.4 for now (I think they've already moved to 16.04, but it might be a while till your average Windows user gets it)

Swift requires cmake > 3.4.3 to build. Ubuntu 14.04 LTS only allows you to install up to version 3.2.2.

All PPAs that I can find are stuck at around version 3.2 or so. The only other option is to install a version of cmake that is supported.

I don't see there as being much of a problem in referring to any cmake version over 3.4.3, however. The docs would be updated if Swift changed its cmake version anyway, so this would be updated too easily

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 7, 2016

Okay, I've done some research, and the best thing to do is to ensure that WSL is running 16.04. I've updated the docs accordingly, removing the references to the old cmake
This is only available to Windows Insiders running on the very latest Windows, so too complicated. The cmake strategy is best, assuming documentation is in the right place

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Nov 7, 2016

I don't (personally) think the landing-page Readme should explicitly mention any platforms other than the officially supported ones. WSL is no more important than Debian or FreeBSD or Android or Cygwin.

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 7, 2016

@jrose-apple I think there should be some sort of specific documentation for platforms (perhaps README is not the best place)

@hughbe hughbe force-pushed the hughbe:wsl-build-instructions branch Nov 7, 2016

@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 7, 2016

Android's documentation lives in docs/Android.md, for example. Perhaps this belongs in docs/Windows.md? That could then be split up in the future, into subsections for WSL, and (hopefully one day) "native" Windows.

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 7, 2016

Also, as of #5113, (copying in @modocache) we actually need to upgrade cmake to support 14.04, so you could argue that this should be in the readme because the readme makes reference to 14.04 above

@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 7, 2016

Oh yeah, good point. I'd be fine with adding CMake update instructions to the README if they're necessary on Ubuntu 16 (which I think is the currently supported version). That probably belongs in a separate pull request, though -- just to separate the discussions between CMake instructions and Windows instructions. :)

@hughbe hughbe force-pushed the hughbe:wsl-build-instructions branch Nov 7, 2016

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 7, 2016

Thanks guys for your comments. I've gotten rid of my changes to the readme, and moved them to docs/Windows.md per @modocache's suggestion.
I do think that we should try linking the docs more from the readme, but thats for a future PR :)

@hughbe hughbe force-pushed the hughbe:wsl-build-instructions branch 2 times, most recently Nov 7, 2016

@hughbe hughbe force-pushed the hughbe:wsl-build-instructions branch to 6f39a37 Nov 8, 2016

@modocache
Copy link
Collaborator

modocache left a comment

Looks good to me, but I had a few nit-picks. I'll merge once you address/choose to ignore those! :)

```

### 2. Install dependencies
Install the developer dependencies needed to compile the Swift project. These are identical to the Ubuntu dependencies, with the addition of `make`

This comment has been minimized.

@modocache

modocache Nov 8, 2016

Collaborator

nit-pick: Add a period at the end of this sentence?

sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-3.6 100
```

### 4. Upgrade CMake

This comment has been minimized.

@modocache

modocache Nov 8, 2016

Collaborator

I like how step three explains why a user needs to upgrade Clang. Could you explain here that the version of CMake included by default on WSL is too old to build Swift, and that at the time of this writing 3.4.3 or greater is required?


### 4. Upgrade CMake
```bash
wget http://www.cmake.org/files/v3.5/cmake-3.5.2.tar.gz

This comment has been minimized.

@modocache

modocache Nov 8, 2016

Collaborator

nit-pick: I'm curious, why not the latest version of CMake? Is 3.5.2 special in some way?

```
### 7. Hello, Windows (Subsystem for Linux)
```bash
cd ./build/Ninja-RelWithDebInfoAssert/swift-linux-x86_64\bin # This path may depend on your build configuration

This comment has been minimized.

@modocache

modocache Nov 8, 2016

Collaborator

Is \bin (backslash) a typo here? Shouldn't it be /bin (forward slash)?

cd ./build/Ninja-RelWithDebInfoAssert/swift-linux-x86_64\bin # This path may depend on your build configuration
vim test.swift
print("Hello, Windows");
:wq

This comment has been minimized.

@modocache

modocache Nov 8, 2016

Collaborator

nit-pick: How about echo 'print("Hello, Windows")' > test.swift? It expresses the same thing in a single line, and doesn't involve using an editor.

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Nov 8, 2016

A little concerned about "Windows.md" being exclusively about WSL. Can you leave stubs for MSVC, at least? cc @compnerd

@igordonxiao

This comment has been minimized.

Copy link

igordonxiao commented Nov 9, 2016

To support Windows PC is very import thing for Swift i think, since Swift is going to expand to service side, there are many developers work on Windows PC, just deploy the Application on Linux like systems, such as Java and JVM based language users. Nowadays, MS(Microsoft)'s open tools can support popular platforms, so Apple do so will be more open, thanks.

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 9, 2016

Thanks guys - I addressed your PR feedback, and added a TODO entry for MSVC

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 10, 2016

Whoops, I didn't push my changes (AFK right now, will do soon)

@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 10, 2016

@swift-ci Please smoke test and merge

@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 10, 2016

@swift-ci please smoke test

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 13, 2016

Just documenting it here. 26 tests unexpectedly fail on Bash on Ubuntu on Windows. I'll investigate them at some point:

Swift-Unit :: Basic/SwiftBasicTests/FileSystem.MoveFileIfDifferentInvalid
Swift(linux-x86_64) :: stdlib/SetTraps.swift
Swift(linux-x86_64) :: stdlib/FloatingPoint.swift.gyb
Swift(linux-x86_64) :: Serialization/write-to-locked-dir.swift
Swift(linux-x86_64) :: Serialization/testability.swift
Swift(linux-x86_64) :: Serialization/resilience.swift
Swift(linux-x86_64) :: Runtime/backtrace.swift
Swift(linux-x86_64) :: Interpreter/return_from_main.swift
Swift(linux-x86_64) :: IRGen/autolink-coff.swift
Swift(linux-x86_64) :: Driver/subcommands.swift
Swift(linux-x86_64) :: Driver/emit-sib-single-file.swift
Swift(linux-x86_64) :: Driver/crash.swift
Swift(linux-x86_64) :: Driver/Dependencies/private-after.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way-provides-before.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way-provides-after.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way-external-delete.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way-depends-before.swift
Swift(linux-x86_64) :: Driver/Dependencies/one-way-depends-after.swift
Swift(linux-x86_64) :: Driver/Dependencies/malformed.swift
Swift(linux-x86_64) :: Driver/Dependencies/malformed-but-valid-yaml.swift
Swift(linux-x86_64) :: Driver/Dependencies/independent.swift
Swift(linux-x86_64) :: Driver/Dependencies/independent-parseable.swift
Swift(linux-x86_64) :: Driver/Dependencies/fail-chained.swift
Swift(linux-x86_64) :: Driver/Dependencies/fail-added.swift
Swift(linux-x86_64) :: Driver/Dependencies/crash-added.swift
@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 13, 2016

@swift-ci please smoke test

@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 13, 2016

@swift-ci please smoke test

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 13, 2016

@modocache I can't seem to trigger the CI (it failed again). Could you fire off another test! thanks

@hughbe hughbe changed the title Update readme for Bash on Ubuntu on Windows Update docs for Bash on Ubuntu on Windows Nov 13, 2016

@CodaFi

This comment has been minimized.

Copy link
Collaborator

CodaFi commented Nov 13, 2016

@swift-ci please smoke test OS X platform.

@compnerd

This comment has been minimized.

Copy link
Collaborator

compnerd commented Nov 13, 2016

I think that adding docs for the MSVC build is a great idea. Ive had multiple people ask how to replicate that, and its somewhat involved as it involves some global environment variables (similar to how Visual Studio works). I don't know how to add a commit on top of this patch set, so I can do that once this is merged.

@compnerd

This comment has been minimized.

Copy link
Collaborator

compnerd commented Nov 13, 2016

BTW, I think it would be nice to squash the commits here since they are addressing review comments rather than making individual improvements to the code base.

@hughbe

This comment has been minimized.

Copy link
Collaborator Author

hughbe commented Nov 13, 2016

You can squash the commits when merging this PR using a GitHub feature

@modocache modocache merged commit 5d22ed4 into apple:master Nov 13, 2016

2 checks passed

Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details
@modocache

This comment has been minimized.

Copy link
Collaborator

modocache commented Nov 13, 2016

Good idea, I squashed the commits into a single one and merged. Thanks, @hughbe!

@hughbe hughbe deleted the hughbe:wsl-build-instructions branch Nov 13, 2016

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Nov 14, 2016

For the record: we generally avoid squashing commits as part of the PR acceptance because sometimes they're logically distinct. In specific cases like this it's okay, but we do want to leave that up to the patch author, and err on the side of merge commits.

(Single-commit PRs can of course be squashed with no loss of information.)

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Nov 14, 2016

Thanks for the new doc, Hugh. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment