Add Initial platform support for Haiku. #11583

return · 2017-08-23T21:35:59Z

This pull request adds initial platform support for building swiftc and stdlib on 64bit Haiku (x86_64-unknown-haiku) using the build-script as part of my GSoC project with the Haiku project.

It also requires this commit, where the OS check is used here. Other than that, it builds and works fine on Haiku and I'm currently adding more support for the test-suite and the foundation libraries.

Resolves SR-NNNN.

slavapestov · 2017-08-23T21:37:18Z

Awesome!

CodaFi · 2017-08-23T21:43:44Z

You'll also want to update this file to get standard library command line support for Haiku.

xwu

Hurray! Just some drive-by formatting nits.

xwu · 2017-08-26T00:33:32Z

stdlib/public/runtime/HaikuPort.cpp

+    image_info i_info;
+    int32 image_cookie = 0;
+    int ret = 0;
+	while(get_next_image_info(team_id, &image_cookie, &i_info) == B_OK)


Nit: Code style in this particular function seems to be very different from everything else, even in this file. Generally, the project uses two spaces instead of four, a space between while or if and the opening paren, and { on the same line as the condition. Also, what's going on with the indentation of this particular line?

xwu · 2017-08-26T00:34:10Z

stdlib/public/Platform/CMakeLists.txt

+      set(GLIBC_ARCH_INCLUDE_PATH "/system/develop/headers/posix")
+      set(GLIBC_SYSROOT_RELATIVE_INCLUDE_PATH "/system/develop/headers/")
+    else()
+	 # Determine the location of glibc headers based on the target.


Nit: indent here and on subsequent lines should be two spaces instead of four.

xwu · 2017-08-26T00:34:52Z

lib/Driver/ToolChains.cpp

+ #if defined(__HAIKU__)
+// For now, passing -use-ld on Haiku doesn't work as swiftc doesn't recognise
+// it. Passing -use-ld= as the argument works fine.
+   Arguments.push_back(context.Args.MakeArgString("-use-ld=" + Linker));


Nit: indent here is missing a space.

xwu · 2017-08-26T00:35:05Z

lib/Basic/Statistic.cpp

  struct rusage RU;
  ::getrusage(RUSAGE_CHILDREN, &RU);
  return RU.ru_maxrss;
 #else
+return 0;


Nit: needs to be indented two spaces.

xwu · 2017-08-26T00:35:34Z

cmake/modules/AddSwift.cmake

@@ -353,6 +353,8 @@ function(_add_variant_link_flags)
      # options. This causes conflicts.
      list(APPEND result "-nostdlib")
    endif()
+   elseif("${LFLAGS_SDK}" STREQUAL "HAIKU")


Nit: indent here is inconsistent with other elseifs.

xwu · 2017-08-26T00:38:02Z

utils/build-script-impl

@@ -425,6 +427,11 @@ function set_build_options_for_host() {
            SWIFT_HOST_VARIANT_SDK="CYGWIN"
            SWIFT_HOST_VARIANT_ARCH="x86_64"
            ;;
+        haiku-x86_64)
+	        SWIFT_HOST_VARIANT="haiku"


Nit: indentation is inconsistent on this line.

xwu · 2017-08-26T00:38:38Z

utils/build-script-impl

@@ -2332,6 +2344,24 @@ for host in "${ALL_HOSTS[@]}"; do
                            -DLLDB_ALLOW_STATIC_BINDINGS=1
                        )
                        ;;
+                     haiku-*)


Nit: indentation is inconsistent as compared to cygwin-*) above and macosx-*) below.

xwu · 2017-08-26T00:39:34Z

utils/swift_build_support/swift_build_support/toolchain.py

@@ -186,6 +186,9 @@ def find_tool(self, *names):
                return found
        return None

+class Haiku(GenericUnix):
+    def __init__(self):
+	super(Haiku, self)


Nit: indentation should be four spaces and not three here.

xwu · 2017-08-26T00:42:14Z

CMakeLists.txt

@@ -536,6 +536,8 @@ else()
    set(SWIFT_HOST_VARIANT_SDK_default "CYGWIN")
  elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
    set(SWIFT_HOST_VARIANT_SDK_default "WINDOWS")
+  elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Haiku")
+	set(SWIFT_HOST_VARIANT_SDK_default "HAIKU")


Nit: indentation here is inconsistent with other lines; use two spaces instead of five.

return · 2017-08-28T16:12:06Z

@xwu @CodaFi I have updated the patches to fit the guidelines, could you please review my changes and check if this is OK to test?

CodaFi · 2017-08-28T16:17:42Z

lib/Driver/ToolChains.cpp

@@ -1578,7 +1578,13 @@ toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
    Linker = getDefaultLinker();
  }
  if (!Linker.empty()) {
+#if defined(__HAIKU__)
+    // For now, passing -use-ld on Haiku doesn't work as swiftc doesn't recognise


Nit: Did you mean -fuse-ld?

@CodaFi This typo has been refactored.

xwu · 2017-08-28T17:35:28Z

lib/Basic/Statistic.cpp

@@ -33,12 +33,16 @@ using namespace llvm::sys;
 static size_t
 getChildrenMaxResidentSetSize() {
 #if defined(HAVE_GETRUSAGE)
+#if !defined(__HAIKU__)


Why is this a separate #if?

@xwu Oops, sorry about that. Would changing it to #if defined(HAVE_GETRUSAGE) && !defined(__HAIKU__) and removing lines 43-45 be preferred?

@xwu I've just refactored this change and it builds fine. Are there any more issues with this PR?

@return I’ve only had a cursory look through the changes, but that’s all I’ve got for now :) Thanks for humoring me.

return · 2017-08-28T20:48:53Z

@CodaFi @xwu Thank you very much for reviewing my changes, are there anymore changes left to do?

CodaFi · 2017-09-04T18:12:23Z

@swift-ci please smoke test

return · 2017-09-04T22:01:09Z

@CodaFi I've already sent a patch for this to LLVM, but this patch isn't included in swift-llvm stable yet. Which is why the build is failing.

korli · 2017-09-05T06:45:01Z

@return I suppose you could cherry-pick apple/swift-llvm@aea1537 in a PR for the stable branch.

return · 2017-09-05T18:54:54Z

@korli I've sent a PR to swift-llvm.

korli · 2017-09-15T16:18:39Z

@swift-ci please smoke test

CodaFi · 2017-09-16T15:10:05Z

@korli PR testing requires privileges. See swift.org

@swift-ci please smoke test

CodaFi · 2017-09-17T19:07:39Z

@swift-ci please smoke test

korli · 2017-09-20T12:07:07Z

@CodaFi thanks for the tests.
@return it seems a change happened on stdlib/public/SwiftShims/LibcShims.h, could you please rebase?

return · 2017-09-21T03:52:15Z

@korli I've rebased my patches and fixed the conflict. @CodaFi Could you please smoke test this commit?

gottesmm · 2017-09-21T03:53:43Z

(Drive by smoke test)

@swift-ci smoke test

korli · 2017-09-21T05:45:21Z

stdlib/public/SwiftShims/LibcShims.h

@@ -121,6 +121,8 @@ typedef unsigned int __swift_thread_key_t;
 typedef int __swift_thread_key_t;
 #elif defined(_WIN32)
 typedef unsigned long __swift_thread_key_t;
+#elif defined(__HAIKU__)
+typedef int __swift_pthread_key_t;


@return it should be __swift_thread_key_t (without p)

@korli Thanks, this has been fixed now.

korli · 2017-09-22T14:46:52Z

LGTM

CodaFi · 2017-09-22T15:58:19Z

@swift-ci please smoke test

CodaFi · 2017-09-22T19:42:06Z

@return If you have no other qualms, this looks good to merge.

return · 2017-09-22T20:28:39Z

@CodaFi These patches are also fine with me.

CodaFi · 2017-09-23T01:06:51Z

⛵️

xwu reviewed Aug 26, 2017

View reviewed changes

return force-pushed the swift-4-haiku-upstream-1 branch from 009e6d2 to 048ebc2 Compare August 27, 2017 17:48

CodaFi reviewed Aug 28, 2017

View reviewed changes

return mentioned this pull request Aug 28, 2017

[GSoC 2017] Porting Swift to Haiku - Final Report haiku/website#124

Merged

return force-pushed the swift-4-haiku-upstream-1 branch from 048ebc2 to 73d2334 Compare August 28, 2017 16:40

return changed the title ~~[WIP] [Do Not Merge] Add Initial platform support for Haiku.~~ Add Initial platform support for Haiku. Aug 28, 2017

xwu reviewed Aug 28, 2017

View reviewed changes

return force-pushed the swift-4-haiku-upstream-1 branch from 73d2334 to 070fdd6 Compare August 28, 2017 18:21

korli approved these changes Sep 4, 2017

View reviewed changes

return mentioned this pull request Sep 5, 2017

[Cherry-picking] Add Haiku Target Triple from LLVM r311153 apple/swift-llvm#58

Closed

return force-pushed the swift-4-haiku-upstream-1 branch from 070fdd6 to 67ace9b Compare September 17, 2017 03:00

return force-pushed the swift-4-haiku-upstream-1 branch from 67ace9b to 0223b3f Compare September 21, 2017 03:38

jessesquires mentioned this pull request Sep 21, 2017

[88] Issue #88 - Sept 21, 2017 SwiftWeekly/swiftweekly.github.io#312

Closed

korli reviewed Sep 21, 2017

View reviewed changes

Add Initial platform support for Haiku.

e7d7ef8

return force-pushed the swift-4-haiku-upstream-1 branch from 0223b3f to e7d7ef8 Compare September 21, 2017 15:27

CodaFi merged commit aee81d2 into swiftlang:master Sep 23, 2017

waddlesplash mentioned this pull request Nov 27, 2017

Haiku support php/php-src#2697

Closed

return deleted the swift-4-haiku-upstream-1 branch January 1, 2018 00:53

Add Initial platform support for Haiku. #11583

Add Initial platform support for Haiku. #11583

Conversation

return commented Aug 23, 2017 • edited Loading

slavapestov commented Aug 23, 2017

CodaFi commented Aug 23, 2017

xwu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

return commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xwu Aug 28, 2017 • edited Loading

Choose a reason for hiding this comment

return commented Aug 28, 2017 • edited Loading

CodaFi commented Sep 4, 2017

return commented Sep 4, 2017

korli commented Sep 5, 2017

return commented Sep 5, 2017

korli commented Sep 15, 2017

CodaFi commented Sep 16, 2017

CodaFi commented Sep 17, 2017

korli commented Sep 20, 2017

return commented Sep 21, 2017

gottesmm commented Sep 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

korli commented Sep 22, 2017

CodaFi commented Sep 22, 2017

CodaFi commented Sep 22, 2017

return commented Sep 22, 2017

CodaFi commented Sep 23, 2017

return commented Aug 23, 2017 •

edited

Loading

xwu Aug 28, 2017 •

edited

Loading

return commented Aug 28, 2017 •

edited

Loading