Partial FreeBSD Support #203

landonf · 2015-12-04T19:33:27Z

This provides a non-controversial baseline for FreeBSD support.

Additional work will be required for the full port, but with this initial work -- plus local work-arounds for linker script behavior and use of pkg-installed libc++ -- I'm able to successfully compete a full Swift build on FreeBSD 10.x.

The alternative to _WITH_GETLINE would be to define _POSIX_C_SOURCE >= 200809.

jckarter · 2015-12-04T20:28:42Z

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

gribozavr · 2015-12-04T20:53:37Z

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

…feedback.

When these diverge, we can use the Unix toolchain as a common baseclass for both.

landonf · 2015-12-04T21:31:32Z

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)

gribozavr · 2015-12-04T22:03:40Z

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

gribozavr · 2015-12-04T22:41:01Z

@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?

landonf · 2015-12-04T22:52:34Z

@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?

gribozavr · 2015-12-04T22:58:42Z

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

gribozavr · 2015-12-04T23:01:36Z

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

jrose-apple · 2015-12-04T23:05:36Z

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. :-)

gribozavr · 2015-12-04T23:07:38Z

I'm dropping SWIFT_ENABLE_TARGET_LINUX in a separate change.

gribozavr · 2015-12-05T00:07:41Z

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

landonf · 2015-12-05T00:33:31Z

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

gribozavr · 2015-12-05T00:35:55Z

Running tests again.

gribozavr · 2015-12-05T01:32:00Z

@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.

landonf · 2015-12-05T01:37:16Z

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

…iple.

This reverts: - Adopt proper freebsd triple as required by Swift's build system. - Revert "Add FreeBSD target_* support. These will be re-introduced in a later patch that brings up the FreeBSD test suite.

landonf · 2015-12-05T03:36:53Z

@gribozavr Great; patchset now matches your change.

archshift · 2015-12-05T05:02:23Z

lib/Driver/Driver.cpp

-      TC = new toolchains::Linux(*this, Target);
+      TC = new toolchains::GenericUnix(*this, Target);
+      break;
+    case llvm::Triple::FreeBSD:


Why not just let this fall through instead of duplicating code?

Fixed via 109d8e6, thanks!

Partial FreeBSD Support

krzyzanowskim · 2015-12-05T09:05:43Z

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: https://github.com/apple/swift/blob/8d9ef80304d7b36e13619ea50e6e76f3ec9221ba/stdlib/private/SwiftPrivatePthreadExtras/PthreadBarriers.swift#L13

gribozavr · 2015-12-05T09:11:08Z

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.

Many of the conflicts stemmed from FreeBSD support, added in swiftlang#203. Conflicts: - CMakeLists.txt - cmake/modules/AddSwift.cmake - lib/Basic/LangOptions.cpp - lib/Driver/ToolChains.cpp - stdlib/public/Glibc/CMakeLists.txt - stdlib/public/core/CMakeLists.txt - stdlib/public/runtime/CMakeLists.txt - stdlib/public/runtime/Casting.cpp - stdlib/public/stubs/CMakeLists.txt - stdlib/public/stubs/LibcShims.cpp - stdlib/public/stubs/Stubs.cpp - test/CMakeLists.txt - utils/build-script-impl

update libpwq and libkqueue submodule versions

update libpwq and libkqueue submodule versions Signed-off-by: Daniel A. Steffen <[email protected]>

Delete the OpenBraceWhitespace rule.

Update hash for IBAnimatableApp to build with Swift 4

- Use durable GUID generation for auto-generation of the GUID. - Combine the platform definitions into a singular source to avoid drift. This avoids having to keep the variants in sync and provides an easier upgrade path. This is the first step towards enabling a non-admin experience for users.

landonf added 6 commits December 4, 2015 12:24

Add basic build system and Driver support for FreeBSD targets/hosts.

a7ec794

Add missing va_list include.

23fee41

FreeBSD hides getline()'s prototype by default.

30080ae

The alternative to _WITH_GETLINE would be to define _POSIX_C_SOURCE >= 200809.

Define malloc_size() support for FreeBSD.

d206dcb

Match the Linux approach of assuming only the 4K NULL page is reserved.

0f6380e

Add FreeBSD Option handling.

6bf5a49

jckarter assigned gribozavr Dec 4, 2015

landonf added 6 commits December 4, 2015 14:01

Drop unnecessary target option, as per gribozavr's feedback.

e8bbe99

Add comment to clarify the empty if block, as per gribozavr's review …

a51494a

…feedback.

Unify the identical Linux/FreeBSD definitions.

d772178

When these diverge, we can use the Unix toolchain as a common baseclass for both.

Merge branch 'master' into landonf/freebsd-patchset-1

0aeef6f

Add a target configuration option test for FreeBSD.

1acab5a

Fix namespace/#endif tokens lost in merge.

49ba19f

landonf added 2 commits December 4, 2015 17:11

Merge branch 'master' into landonf/freebsd-patchset-1

83b706d

Adopt suggested 'GenericUnix' toolchain class name.

daa97e0

landonf added 4 commits December 4, 2015 18:39

Apply patch provided by gribozavr; swift -target requires a proper tr…

c671947

…iple.

Adopt proper freebsd triple as required by Swift's build system.

527fb68

Add FreeBSD target_* support.

395ae26

Revert changes not needed for test passage on supported platforms.

c4daece

This reverts: - Adopt proper freebsd triple as required by Swift's build system. - Revert "Add FreeBSD target_* support. These will be re-introduced in a later patch that brings up the FreeBSD test suite.

archshift reviewed Dec 5, 2015
View reviewed changes

gribozavr added a commit that referenced this pull request Dec 5, 2015

Merge pull request #203 from landonf/landonf/freebsd-patchset-1

16de5d1

Partial FreeBSD Support

gribozavr merged commit 16de5d1 into swiftlang:master Dec 5, 2015

modocache mentioned this pull request Jan 7, 2016

Merge apple/swift master (at 1302539a) SwiftAndroid/swift#11

Closed

slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018

Merge pull request swiftlang#203 from dgrove-oss/update-submodules

c95febb

update libpwq and libkqueue submodule versions

slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018

Merge pull request swiftlang#203 from dgrove-oss/update-submodules

33772ec

update libpwq and libkqueue submodule versions Signed-off-by: Daniel A. Steffen <[email protected]>

dabelknap added a commit to dabelknap/swift that referenced this pull request Jan 18, 2019

Merge pull request swiftlang#203 from google/format-delete-open-brace-ws

d22a24d

Delete the OpenBraceWhitespace rule.

freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022

Merge pull request swiftlang#203 from tkremenek/IBAnimatableApp-4.0

37a8158

Update hash for IBAnimatableApp to build with Swift 4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial FreeBSD Support #203

Partial FreeBSD Support #203

landonf commented Dec 4, 2015

jckarter commented Dec 4, 2015

gribozavr commented Dec 4, 2015

landonf commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 4, 2015

landonf commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 4, 2015

jrose-apple commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 5, 2015

landonf commented Dec 5, 2015

gribozavr commented Dec 5, 2015

gribozavr commented Dec 5, 2015

landonf commented Dec 5, 2015

landonf commented Dec 5, 2015

archshift Dec 5, 2015

gribozavr Dec 5, 2015

krzyzanowskim commented Dec 5, 2015

gribozavr commented Dec 5, 2015

Partial FreeBSD Support #203

Partial FreeBSD Support #203

Conversation

landonf commented Dec 4, 2015

jckarter commented Dec 4, 2015

gribozavr commented Dec 4, 2015

landonf commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 4, 2015

landonf commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 4, 2015

jrose-apple commented Dec 4, 2015

gribozavr commented Dec 4, 2015

gribozavr commented Dec 5, 2015

landonf commented Dec 5, 2015

gribozavr commented Dec 5, 2015

gribozavr commented Dec 5, 2015

landonf commented Dec 5, 2015

landonf commented Dec 5, 2015

archshift Dec 5, 2015

Choose a reason for hiding this comment

gribozavr Dec 5, 2015

Choose a reason for hiding this comment

krzyzanowskim commented Dec 5, 2015

gribozavr commented Dec 5, 2015