-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add support for the GNUstep Objective-C runtime #27
Conversation
Hey @ngrewe, thanks for this! Really excited that you're interested! I'll take a look over this when I get a chance, have to do some research first on GNUstep since I've never worked with it before :) My concerns would be:
Definitely still think we should have this! It's cool to make the objc crate usable on more platforms. |
Yes. That's a pretty valid concern. We've all been to #ifdef hell, haven't we? I didn't get around to playing with rust-objc-foundation yet, so I can't really say anything about that (in the best case, it just requires a single `#[cfg()]' to link against gnustep-base instead of Foundation.framework), but for the other components it's mostly straightfoward:
I'm not very familiar with travis, but I guess I can manage to set up a build script that pulls the runtime and builds the Linux version of the crate. Building gnustep-base is a bit more tricky (because dependencies), so what I'm currently looking at is providing a mock-implementation of NSObject in rust-objc so that the tests can run without requiring to link something else. |
It seems that it isn't possible. The link statement for rustc is emitted unconditionally. I now have a patched version of the gcc crate that makes that configurable.
That works well. The code for that is in a separate branch, because it requires my gcc-rs patch. Let's see whether I can upstream that. (It's difficult to write tests for, unfortunately…) |
My proposed changes were merged into gcc-rs. I've updated the PR to no longer depend on gnustep-base for the test and to have a travis build script that builds the code for Linux and Mac OS X with stable, beta, and nightly Rust. |
Dang @ngrewe you've been crazy productive, thanks again :) I'm traveling again this weekend but I promise I haven't forgotten about this PR! At the latest I should be able to catch up on this next weekend. |
@@ -31,7 +30,7 @@ fn msg_send_fn<R: Any>() -> unsafe extern fn(*mut Object, Sel, ...) -> R { | |||
} | |||
} | |||
|
|||
#[cfg(target_arch = "x86")] | |||
#[cfg(all(target_arch = "x86", not(feature = "gnustep_runtime")))] | |||
fn msg_send_super_fn<R: Any>() -> unsafe extern fn(*mut Object, Sel, ...) -> R { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this have to be cfg
'd out? It didn't look like any runtime methods were removed in gnustep, so I'd expect it to still compile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GNUstep runtime doesn't implement sending messages to super via objc_msgSendSuper
. The GNUstep-specific implementation for sending messages to super is at ll. 182–194.
Sorry for the delay @ngrewe, I finally read up a bit on GNUstep and looked over this PR more in-depth. I've also added Travis support for this repo, could you rebase your changes on top of master to trigger a Travis run so that we can verify I've set it up properly? :) |
|
||
[features] | ||
exception = ["objc_exception"] | ||
gnustep_runtime = [ "gcc" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I'd be okay with calling this feature just gnustep
if we want to save a few characters, it seems just as descriptive. Your thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
Thanks for taking the time to review the PR! I hope I have addressed all your comments. Please let me know what you think. Cheers, Niels |
Hi @SSheldon! I hope you don't mind if I poke you once more about this PR? Do you think there is anything else I should look into? Cheers, Niels |
Hey @ngrewe! Sorry for falling off the face of the earth, I haven't been doing too much with this lib recently. I was planning to pull your change in for the 0.2 release (also dropping libc and adding optional verification of msgSend arguments based on encoding!), and some of those changes are in master so now's as good a time as any :) If you've got a chance and want to rebase and squash your commits on master, then I can just fast forward it straight to master. |
Support for the GNUstep Objective-C runtime Restore accidentally lost attribute Cross-platform support for message sends with the GNUstep runtime. Add feature gates for the platform specific objc_msgSend implementation used by the GNUstep runtime (mips not currently supported because I'm unaware of the calling conventions). In all other cases, fall back to the cross-platform two-stage message sending. This now works and passes some of the tests (I had a buggy old gcc version yesterday, it seems). Every test that assumes NSObject to be present fails, though because that is not part of the GNUstep runtime. We'll either have to link gnustep-base for the tests to run properly, or provide stub implementation. Fix calling objc_slot_lookup_super(), which had the argument type wrong. Trick the linker into linking gnustep-base to pull in NSObject, eventhough we never reference the symbol for it. Also a bit of documentation. Fix libobjc2 repository location. Satisfy test dependencies using a stub NSObject implementation. Requires a patched gcc crate at the moment. Word Update to track proposed gcc-rs API Changes to the gcc crate were merged (cf. rust-lang/cc-rs#54) Experiment with travis builds Slim down a bit Shell script syntax is hard More shell script tweaks Tweak libobjc2 install location Stage libobjc2 into a local directory Conditionalize features, fix missing ‘;’ again. GNUstep base is no longer required for running tests. Fix gcc-rs revision Depend on a specific gcc-rs revision from github until a release includes the changes needed. Update dependencies to the released gcc-rs version. Exclude .travis.yml from publishing Restore original arch (aarch64 was incorrectly replaced with arm) Move NSObject dependency for tests with the GNUstep runtime into a sub-crate Rename ‘gnustep_runtime’ to ‘gnustep’ for brevity.
Not a problem. I know how things go sometimes :-)
I've never used git rebase before, so this was actually an amazing learning experience. The result looks plausible to me, so there's hope that I didn't mess it up ;-) Again, many thanks for taking the time to look at this! Cheers, Niels |
@ngrewe glad we've both gotten to learn throughout this :P I was playing around with this code one last time today and poking at some tests. I tried to see what would happen if I did After much sleuthing, what I'm basically wondering is: what's the difference between gnustep/libobjc2 and gcc/libobjc? The gcc libobjc seems to be the one that's available from package managers. The GNUstep libobjc2 seems to be backwards compatible with the gcc libobjc, so should we be targeting the gcc version to reach the widest userbase? Or is no one using the gcc version anymore? I have no idea what the state of this is on Linux, so I'd appreciate your knowledge here! |
Specifically, the symbols that weren't available in the gcc libobjc in my testing are:
|
Okay, I found this: http://wiki.gnustep.org/index.php/ObjC2_FAQ I think I'm understanding things now. Just like Apple's runtime is now Objective-C 2.0, there is a version 2.0 of the GNUstep runtime, except the default version of GNUstep in gcc is 1.0 because the 2.0 runtime is "not currently considered production ready". So, given that this crate works with Apple's current version of the runtime (Objective-C 2) it might make sense to support the libobjc2. However, if that's used pretty rarely maybe it'd be more worth our time to support the GNUstep Objective-C 1 runtime. Thoughts? |
You are right that we have some pretty confusing ecosystem fragmentation here. The old/legacy runtime (‘gcc runtime’) does not support any features of ‘modern’ Objective-C, it's also effectively been unmaintained since 2011, with only the minimal amount of work going into it to prevent it from bitrotting in the gcc repository. I'm most concerned about that it doesn't have any of the ARC hooks which are required to implement Also, that wiki page is unfortunately really outdated. The sentence ’not currently considered production ready’ pretty much reflects the state of 2009. I would definitely consider the new runtime to be quite mature by now. It's even the default Objective-C runtime shipped by FreeBSD. That you can only get the old runtime from the default Linux package repositories is very unfortunate, but I think that's the result of not enough people caring about Linux packaging and some weird licensing mismatch (the GCC runtime is LGPL, while the GNUstep one is MIT licensed). |
@ngrewe got it, that makes sense. Enough of my pondering, I've merged this! Still learning a lot about GNUstep, so I might have more crazy questions but with it merged now it's easy to iterate on :) |
Cool! 😃 Many thanks. |
Hi!
I've fiddled with a few things and gotten the bindings to work with the GNUstep Objective-C runtime, so that they can also be used for interop between Rust and Objective-C code on non-Apple platforms. It's controlled by a new feature called
gnustep_runtime
,This mainly involved adding support for the slightly different message sending mechanism. Also getting the tests back into working order proved a bit difficult because if you're not on Mac OS or iOS you usually don't have NSObject available unless you link a Foundation implementation. I've made it so that the bindings will build without one, but the tests will need to link the GNUstep base library (Foundation equivalent) in order to run.
There's also an outstanding bug in the runtime that I found thanks to the unit tests in rust-objc ;-)
Also, I've tested on Mac OS for regressions and things should continue to work there as expected.