Skip to content
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

Use CoreFoundation from Swift #381

Closed
wants to merge 43 commits into from

Conversation

eleanxr
Copy link

@eleanxr eleanxr commented Nov 12, 2022

Addresses #82 .

This PR replaces the contents of Frameworks/CoreFoundation with the contents of the CoreFoundation subdirectory for the Swift 5.7.1 release (https://github.com/apple/swift-corelibs-foundation/tree/swift-5.7.1-RELEASE/CoreFoundation).

In my fork, I tagged the version corresponding to the unmodified import to serve as a branch point for future updates (https://github.com/deleanor/ravynos/tree/import-swift-cf-swift-5.7-RELEASE).

To see the specific changes to the Swift CF implementation that were required to work with the current RavynOS frameworks, it's probably easiest to view the diff in the CoreFoundation directory against the import tag: eleanxr/ravynos@import-swift-cf-swift-5.7-RELEASE...deleanor:ravynos:swift-cf-RunLoopWithKevent

Considerations:

  • As noted in the issue, I applied the upstream patch to use Kevent within CFRunLoop mentioned in Swift build on FreeBSD #209 (Omnibus to implement CFRunLoop with kevent. swiftlang/swift-corelibs-foundation#3004).
  • I attempted to make the minimal set of changes to the CF implementation to aid in future updates.
  • There are many preprocessor guards of the form #if TARGET_OS_MAC && !__RAVYNOS__. This is because ravynos/include/TargetConditionals.h ends up defining TARGET_OS_MAC. It may be beneficial to either augment that header to have a TARGET_OS_RAVYNOS or in the longer term make RavynOS fully compatible with TARGET_OS_MAC. For now, we defer to the alternative TARGET_OS_BSD branches where necessary by adding a || __RAVYNOS__ where they align.
  • As noted in the issue, a small shim was necessary in Foundation to handle cases where Foundation objects were passed into CF and CF is expecting type information via CFRuntimeBase.
  • The Swift-provided CoreFoundation.h header includes CFXMLNode.h and CFXMLParser.h when the deployment runtime is not Swift, but those files don't exist in the repository as far as I can tell. I conditionally excluded those for now, and I had to exclude CFXMLInterface for the same reason. There are versions of those files in Apple's open source CF if we want to try and adapt those later.
  • I've been able to test very little. I can currently only test the beginning of WindowServer initialization, but it makes it to the same place on my VM that the main branch makes it to. I'm not sure what other executables to test, but I'm open to suggestions. I also wrote a simple test for CFRunLoop that appears to work (https://github.com/deleanor/ravynos-cf-test). I'd love to hear additional suggestions for testing, but short of writing a large test suite to go with this I'm not sure what else I can do in that area at the moment. Future options include using the Foundation test suite that ships with swift-corelibs-foundation, but that would be down the road because it requires Swift language support.

Need to re-evaluate everything in this commit.
-Wthread-safety-anaysis
-Wcast-qual
-Wunused-parameter
https://github.com/apple/swift-corelib-foundation/pull/3004

Subject: [PATCH] [runloop] Set up some symbols for generic code.

CFRunLoop has a recurring abstraction for platform-specific code, but it
is a little leaky. Plug these leaks: ensure `MACH_PORT_NULL` in the
generic, non-platform context is rewritten as `CFPORT_NULL`, and ensure
that `kern_return_t` and `KERN_SUCCESS` are defined properly.

[runloop] Remind porter these stubs are required.

New platforms must define these types and functions; it is incorrect to
not have #else cases here.

[runloop] Implement runloop abstraction for BSD.

[runloop] Ensure timing arithmetic is done in ns.

Fixes SR-14288.

[runloop] Avoid pthread_main_np on OpenBSD.

During runloop testing, log messages suggest that the runloop thinks the
main thread has exited and certain runloop operations don't actually
proceed. The flag for denoting when the main thread has exited is set in
the thread-specific data destructor `__CFTSDFinalize`.

This function detects whether the thread in question is the main thread
via `pthread_main_np`, and has been observed in debugging this problem
to return 1 on apparently non-main threads on OpenBSD. This obviously
will cause the main thread exited flag to be erroneously set when
non-main threads complete their work and dispose of thread-specific
data.

Instead, use the existing `_CFMainPThread` symbol set in
`__CFInitialize` to determine the main thread. I am not sure whether the
platform `pthread_main_np` is at fault here, but since this workaround
makes the tests pass, let's use it.
I'm not sure what happened here. I had a build before this change,
but these failed when I tried to build after a clean.
@eleanxr
Copy link
Author

eleanxr commented Nov 23, 2022

I think I was probably overly optimistic opening this PR. I've been reading more documentation/code for toll-free bridging to try and figure out how to implement it correctly in a follow-on issue and I'm pretty sure the other frameworks are going to break if this were to get merged. I've found a few locations where frameworks are assuming that they can cast freely between some of the CF and Foundation types. For example, CGImageSource casts from a CFURL to a NSURL to pass into O2ImageSource. So I think this is not ready yet until I can either get bridging in or find a more complete workaround.

@eleanxr eleanxr closed this Nov 23, 2022
@lin72h
Copy link

lin72h commented Nov 24, 2022

Good work takes time. Thanks for your persistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants