Nice! @gribozavr is probably the best person to review the build system changes.

@landonf Great work! Please update the pull request per comments above. Which systems have you tested this branch on?

Thanks for the feedback; I believe I've integrated all the requested changes. I also moved ahead with a common Unix toolchain class pending additional comments on a preferred alternative.

I've only tested locally against x86-64-freebsd10.2 (-STABLE)

I'll run the tests on OS X and Linux, while waiting for feedback from @jrose-apple .

@landonf The OS X build passed all tests.

The new test fails on Linux.

FAIL: Swift :: BuildConfigurations/x64FreeBSDTarget.swift (141 of 7260)
******************** TEST 'Swift :: BuildConfigurations/x64FreeBSDTarget.swift' FAILED ********************
Script:
--
'/home/gribozavr/src.public/build/Ninja-ReleaseAssert/swift-linux-x86_64/bin/swift' -frontend -module-cache-path '/tmp/swift-testsuite-clang-module-cacheQ2TPZE' -disable-objc-attr-requires-foundation-module -parse /home/gribozavr/src.public/swift/test/BuildConfigurations/x64FreeBSDTarget.swift -verify -D FOO -D BAR -target x86_64-freebsd10 -disable-objc-interop -D FOO -parse-stdlib
'/home/gribozavr/src.public/build/Ninja-ReleaseAssert/swift-linux-x86_64/bin/swift-ide-test' -module-cache-path '/tmp/swift-testsuite-clang-module-cacheQ2TPZE' -completion-cache-path '/tmp/swift-testsuite-completion-cachee5rxPO' -test-input-complete -source-filename=/home/gribozavr/src.public/swift/test/BuildConfigurations/x64FreeBSDTarget.swift -target x86_64-freebsd10
--
Exit Code: 1

Command Output (stderr):
--
<unknown>:0: error: unsupported target OS: ''

--

********************

I think this is because the FreeBSD toolchain is being disabled on Linux with a #if in the C++ source. Could you take a look?

@gribozavr My fault for dropping SWIFT_ENABLE_TARGET_FREEBSD without verifying that it didn't break lib/Driver/CMakeLists.txt target define handling.

How did you want to handle this for Linux? Should we just enable both targets by default? Or should I bring back SWIFT_ENABLE_TARGET_FREEBSD and make the tests conditional on the targets being enabled?

@landonf I'd prefer that we drop the Linux option, too.

@landonf In fact, I'll do it now, give me an hour.

In the spirit of incremental development, can we do these as separate pull requests? Drop SWIFT_ENABLE_TARGET_LINUX, rename toolchain and fix-up the Driver library in preparation for the main change, then actually add the support. Or whatever units make sense to you. (Feel free to rewrite history ahead of the merges.)

Aside: Sorry for the trouble, Landon. I'm/We're very happy to have you working on this even as I'm pushing for this extra process. Having small, logically distinct commits is especially important if we ever need to bisect a failure—it's better to be able to isolate a specific change rather than potentially having to revert the whole merge commit. Also, once a change is in master, others are held accountable for not breaking it. :-)

I'm dropping SWIFT_ENABLE_TARGET_LINUX in a separate change.

Removed SWIFT_ENABLE_TARGET_LINUX in a3c92e1. Unfortunately this created merge conflicts. Sorry for the churn @landonf .

@gribozavr Thanks! No problem; I've merged in the changes (and adopted @jrose-apple's suggested "GenericUnix").

Running tests again.

@landonf The -target option accepts a triple. Please apply a patch like this to your branch (use a more correct triple if required):

diff --git a/test/BuildConfigurations/x64FreeBSDTarget.swift b/test/BuildConfigurations/x64FreeBSDTarget.swift
index 619d4f5..284060f 100644
--- a/test/BuildConfigurations/x64FreeBSDTarget.swift
+++ b/test/BuildConfigurations/x64FreeBSDTarget.swift
@@ -1,5 +1,5 @@
-// RUN: %swift -parse %s -verify -D FOO -D BAR -target x86_64-freebsd10 -disable-objc-interop -D FOO -parse-stdlib
-// RUN: %swift-ide-test -test-input-complete -source-filename=%s -target x86_64-freebsd10
+// RUN: %swift -parse %s -verify -D FOO -D BAR -target x86_64-unknown-freebsd10 -disable-objc-interop -D FOO -parse-stdlib
+// RUN: %swift-ide-test -test-input-complete -source-filename=%s -target x86_64-unknown-freebsd10

 #if arch(x86_64) && os(FreeBSD) && _runtime(_Native)
 class C {}

With this patch, tests pass on Linux and OS X.

Thanks, will do; I'm going to bring up a Linux machine so I can properly test locally.

@gribozavr Great; patchset now matches your change.

Does this change means that most #if os(Linux) should be now #if os(Linux) || os(FreeBSD) and all else conditions are undefined now?

#if os(OSX) || os(iOS) || os(watchOS) || os(tvOS)
import Darwin
#elseif os(Linux)
import Glibc
#endif

ref:

Yes, all these places need to be audited (we tried to write them in such a way that a new platform would cause build breakage rather than silently choosing some behavior), but not all of them would become os(Linux) || os(FreeBSD). For example, FreeBSD does not use Glibc, it uses BSD libc, so the module likely wouldn't be called Glibc.