Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add FFI::Platform::IS_MAC constant #2096

Closed
wants to merge 3 commits into from

5 participants

@sempervictus

MRI FFI contains a number of constants missing from RBX. This
commit adds IS_MAC to the case statement defining Platform const.

TODO: add IS_GNU, IS_LINUX, IS_FREEBSD, IS_OPENBSD, IS_BSD

this should resolve #2095

@jc00ke
Owner

Are there specs for these constants?

@dbussink
Owner

If we're adding these constants, we should also include specs. I'm also not a fan of implementing this half. We should also add the other constants with specs and not leave that as a TODO.

@brixen
Owner

@sempervictus you could look at this commit for examples on the specs.

RageLtMan added some commits
RageLtMan Add FFI::Platform::IS_MAC constant
MRI FFI contains a number of constants missing from RBX. This
commit adds IS_MAC to the case statement defining Platform const.

TODO: add IS_GNU, IS_LINUX, IS_FREEBSD, IS_OPENBSD, IS_BSD
04c3b6e
RageLtMan add spec for FFI::Platform::IS_MAC 6c27466
@sempervictus

If this spec fits the bill lets commit IS_MAC in this PR and i'll add appropriate requests for Linux and other BSD based platforms in a separate PR.

spec/ruby/optional/ffi/platform_spec.rb
@@ -40,6 +40,26 @@
end
end
+describe "FFI::Platform::IS_MAC" do
+ platform_is :linux do
+ it "returns false" do
+ FFI::Platform::IS_MAC.should == false
+ end
+ end
+
+ platform_is :windows do
+ it "returns true" do
+ FFI::Platform::IS_MAC.should == false
@brixen Owner
brixen added a note

The spec description ("returns true") and the expectation disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spec/ruby/optional/ffi/platform_spec.rb
((4 lines not shown))
+describe "FFI::Platform::IS_MAC" do
+ platform_is :linux do
+ it "returns false" do
+ FFI::Platform::IS_MAC.should == false
+ end
+ end
+
+ platform_is :windows do
+ it "returns true" do
+ FFI::Platform::IS_MAC.should == false
+ end
+ end
+
+ platform_is :darwin do
+ it "returns false" do
+ FFI::Platform::IS_MAC.should == true
@brixen Owner
brixen added a note

Same problem here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
RageLtMan Add platform constants for Linux and BSD
This commit creates constants and specs for the remaining
FFI::Platform::IS_LINUX constant.

Additional constants have been added for compatibility with MRI's
FFI gem: IS_GNU, NAME, IS_FREEBSD, IS_OPENBSD, IS_BSD.

BSD constants are pointless since RBX does not currently check for
BSD platforms other than OSX. IS_BSD is the only constant which can
be true at the moment since IS_MAC can be true as well.
IS_GNU should be reviewed and potentially spec'ed as it currently
checks definition of FFI::Library::LIBC for its boolean value which
may not be the correct approach. MRI implementation checked for
the definition of "GNU_LIBC" constant which appears analogous to
our FFI::Library::LIBC.
0f12495
@sempervictus

Thank you sir - nice to know i am actually capable of screwing that up :).

Fixed and added the remaining constants to this PR since aside from IS_LINUX the rest seem pointless at present. Does my take on IS_GNU work, or should i rewire that and spec it somehow?

@subwindow subwindow commented on the diff
kernel/platform/ffi.rb
((25 lines not shown))
OS = 'linux'
end
ARCH = Rubinius::CPU
+ NAME = "#{ARCH}-#{OS}"
+ IS_GNU = defined?(FFI::Library::LIBC)
+ IS_FREEBSD = OS == "freebsd"
@subwindow Collaborator

I don't think this is right, because IS_LINUX and IS_.*BSD can be true at the same time.

I would suggest adding Rubinius.bsd? to the case statement and doing the distinguishing between free/open inside that block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sempervictus

Thi The only logic change i made was to define IS_GNU via FFI::Library::LIBC, the logic for the BSD definitions comes from MRI's FFI gem (though the validations are a bit different). Not sure about the semantics of BSD being Linux, though i'm pretty sure that they are mutually exclusive definitions.

This may all be a moot point if Brixen gets FFI to work in tandem with Rubinius's low level implementation as we'll just have access to their constants.

@brixen
Owner

We now bundle a 1.2.0 version of the FFI gem, however, there is a bug in RVM that omits the pre-installed gems. So if using RVM, use the recently released 1.3.0 FFI gem which builds correctly on Rubinius.

@brixen brixen closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 16, 2012
  1. Add FFI::Platform::IS_MAC constant

    RageLtMan authored
    MRI FFI contains a number of constants missing from RBX. This
    commit adds IS_MAC to the case statement defining Platform const.
    
    TODO: add IS_GNU, IS_LINUX, IS_FREEBSD, IS_OPENBSD, IS_BSD
  2. add spec for FFI::Platform::IS_MAC

    RageLtMan authored
  3. Add platform constants for Linux and BSD

    RageLtMan authored
    This commit creates constants and specs for the remaining
    FFI::Platform::IS_LINUX constant.
    
    Additional constants have been added for compatibility with MRI's
    FFI gem: IS_GNU, NAME, IS_FREEBSD, IS_OPENBSD, IS_BSD.
    
    BSD constants are pointless since RBX does not currently check for
    BSD platforms other than OSX. IS_BSD is the only constant which can
    be true at the moment since IS_MAC can be true as well.
    IS_GNU should be reviewed and potentially spec'ed as it currently
    checks definition of FFI::Library::LIBC for its boolean value which
    may not be the correct approach. MRI implementation checked for
    the definition of "GNU_LIBC" constant which appears analogous to
    our FFI::Library::LIBC.
This page is out of date. Refresh to see the latest.
Showing with 53 additions and 0 deletions.
  1. +13 −0 kernel/platform/ffi.rb
  2. +40 −0 spec/ruby/optional/ffi/platform_spec.rb
View
13 kernel/platform/ffi.rb
@@ -223,25 +223,38 @@ def initialize(struct)
##
# Namespace for holding platform-specific C constants.
+##
module FFI::Platform
case
when Rubinius.windows?
LIBSUFFIX = "dll"
IS_WINDOWS = true
+ IS_MAC = false
+ IS_LINUX = false
OS = 'windows'
when Rubinius.darwin?
LIBSUFFIX = "dylib"
IS_WINDOWS = false
+ IS_MAC = true
+ IS_LINUX = false
OS = 'darwin'
else
LIBSUFFIX = "so"
IS_WINDOWS = false
+ IS_MAC = false
+ IS_LINUX = true
OS = 'linux'
end
ARCH = Rubinius::CPU
+ NAME = "#{ARCH}-#{OS}"
+ IS_GNU = defined?(FFI::Library::LIBC)
+ IS_FREEBSD = OS == "freebsd"
@subwindow Collaborator

I don't think this is right, because IS_LINUX and IS_.*BSD can be true at the same time.

I would suggest adding Rubinius.bsd? to the case statement and doing the distinguishing between free/open inside that block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ IS_OPENBSD = OS == "openbsd"
+ IS_BSD = IS_MAC || IS_FREEBSD || IS_OPENBSD
+
# ruby-ffi compatible
LONG_SIZE = Rubinius::SIZEOF_LONG * 8
ADDRESS_SIZE = Rubinius::WORDSIZE
View
40 spec/ruby/optional/ffi/platform_spec.rb
@@ -40,6 +40,46 @@
end
end
+describe "FFI::Platform::IS_MAC" do
+ platform_is :linux do
+ it "returns false" do
+ FFI::Platform::IS_MAC.should == false
+ end
+ end
+
+ platform_is :windows do
+ it "returns false" do
+ FFI::Platform::IS_MAC.should == false
+ end
+ end
+
+ platform_is :darwin do
+ it "returns true" do
+ FFI::Platform::IS_MAC.should == true
+ end
+ end
+end
+
+describe "FFI::Platform::IS_LINUX" do
+ platform_is :linux do
+ it "returns true" do
+ FFI::Platform::IS_LINUX.should == true
+ end
+ end
+
+ platform_is :windows do
+ it "returns false" do
+ FFI::Platform::IS_LINUX.should == false
+ end
+ end
+
+ platform_is :darwin do
+ it "returns false" do
+ FFI::Platform::IS_LINUX.should == false
+ end
+ end
+end
+
describe "FFI::Platform::ARCH" do
it "returns the architecture type" do
FFI::Platform::ARCH.should == Rubinius::CPU
Something went wrong with that request. Please try again.