Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-2219

Thrift gem fails to build on OS X Mavericks with 1.9.3 rubies

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.9.1
    • 0.9.2
    • Ruby - Library
    • None
    • OS X Mavericks (GM), ruby 1.9.3-p448 built with rbenv/ruby-build

    Description

      (Preface, this issue is for the unreleased OS X Mavericks, so understand if it's not fixable now, just wanted to bring it up.)

      I'm unable install the 0.9.1 thrift gem on my GM OS X Mavericks system with ruby 1.9.3-p448. I get the error `extconf.rb:25: Use RbConfig instead of obsolete and deprecated Config.` along with some strlcopy errors.

      Strack trace is at (https://gist.github.com/chrismanderson/6834809)

      Oddly, I CAN install the 0.9.1 gem with both 2.0.0-p247 and ree-1.8.7-2011.12. I also tried 1.9.3-p194 and p429 and got the same error.

      Attachments

        Issue Links

          Activity

            ccutrer Cody Cutrer added a comment -

            note that Mavericks is now GM, so essentially released

            ccutrer Cody Cutrer added a comment - note that Mavericks is now GM, so essentially released
            chrismanderson Chris Anderson added a comment -

            Did a little more digging - on a fresh 10.9 system, with only rbenv, ruby-build, and 1.9.3-p448 installed via homebrew, the gem builds. Once you install the Xcode command line tools however, the install fails with the above error. GCC looks to be the same version before/after, so something else is gumming up the works.

            chrismanderson Chris Anderson added a comment - Did a little more digging - on a fresh 10.9 system, with only rbenv, ruby-build, and 1.9.3-p448 installed via homebrew, the gem builds. Once you install the Xcode command line tools however, the install fails with the above error. GCC looks to be the same version before/after, so something else is gumming up the works.
            nham Ben Nham added a comment - - edited

            The issue is that in Mavericks, strlcpy is now defined as a macro if you install the SDK. This conflicts with the duplicate declaration of strlcpy as a function in strlcpy.h. The fix should be simple: don't re-declare the strlcpy prototype in strlcpy.h. Just remove that line, since <string.h> is guaranteed to have declared strlcpy declared already--this is tested for already in lib/rb/ext/extconf.rb via the call to have_func("strlcpy", "string.h").

            As a temporary fix for folks using gems, you can try setting the #define'ing the _FORTIFY_SOURCE macro to 0 to make strlcpy a function rather than a macro:

            $ gem install thrift -- --with-cppflags='-D_FORTIFY_SOURCE=0'
            

            or if you're using bundler, you can set the cflags for thrift locally via:

            $ bundle config build.thrift --with-cppflags='-D_FORTIFY_SOURCE=0'
            

            The workaround uses with-cppflags rather than with-cflags since since thrift's extconf.rb overwrites the $cflags global with its own stuff, but leaves $cppflags alone. Ruby's mkmf.rb passes both $cflags and $cppflags along to the compiler so this slightly hacky workaround seems to work.

            nham Ben Nham added a comment - - edited The issue is that in Mavericks, strlcpy is now defined as a macro if you install the SDK. This conflicts with the duplicate declaration of strlcpy as a function in strlcpy.h. The fix should be simple: don't re-declare the strlcpy prototype in strlcpy.h. Just remove that line, since <string.h> is guaranteed to have declared strlcpy declared already--this is tested for already in lib/rb/ext/extconf.rb via the call to have_func("strlcpy", "string.h"). As a temporary fix for folks using gems, you can try setting the #define'ing the _FORTIFY_SOURCE macro to 0 to make strlcpy a function rather than a macro: $ gem install thrift -- --with-cppflags='-D_FORTIFY_SOURCE=0' or if you're using bundler, you can set the cflags for thrift locally via: $ bundle config build.thrift --with-cppflags='-D_FORTIFY_SOURCE=0' The workaround uses with-cppflags rather than with-cflags since since thrift's extconf.rb overwrites the $cflags global with its own stuff, but leaves $cppflags alone. Ruby's mkmf.rb passes both $cflags and $cppflags along to the compiler so this slightly hacky workaround seems to work.
            mroth Michael Roth added a comment -

            Ben Nham's hack works!

            mroth Michael Roth added a comment - Ben Nham's hack works!
            antifuchs Andreas Fuchs added a comment - - edited

            Looks like Ben Nham's diagnosis is correct: The declaration of strlcpy there causes badness. Removing that #else branch on the strlcpy.h ifdef seems to do the trick. This is essentially the same change as https://github.com/nearbuy/thrift/commit/b20535e9d58e0f099338f690bab856ceb0041e63, but I decided to remove the now-useless #else as well.

            I tested this in 10.9 and on Linux, and the gem builds there. I'll test on 10.7 later today (as soon as I get access to a machine running it); I hope that will give us enough certainty this is the correct change to make.

            (This is my first submission to the Apache JIRA; please let me know if I screwed anything up )

            antifuchs Andreas Fuchs added a comment - - edited Looks like Ben Nham's diagnosis is correct: The declaration of strlcpy there causes badness. Removing that #else branch on the strlcpy.h ifdef seems to do the trick. This is essentially the same change as https://github.com/nearbuy/thrift/commit/b20535e9d58e0f099338f690bab856ceb0041e63 , but I decided to remove the now-useless #else as well. I tested this in 10.9 and on Linux, and the gem builds there. I'll test on 10.7 later today (as soon as I get access to a machine running it); I hope that will give us enough certainty this is the correct change to make. (This is my first submission to the Apache JIRA; please let me know if I screwed anything up )
            antifuchs Andreas Fuchs added a comment -

            I just tested the previous patch on 10.7 and discovered it was reversed. Oops. Attaching a patch that goes in the correct direction.

            In my tests works on both 10.9 and 10.7. I hope we can get this committed so the next release comes with a ruby gem that can be installed on 10.9.

            antifuchs Andreas Fuchs added a comment - I just tested the previous patch on 10.7 and discovered it was reversed. Oops. Attaching a patch that goes in the correct direction. In my tests works on both 10.9 and 10.7. I hope we can get this committed so the next release comes with a ruby gem that can be installed on 10.9.
            jfarrell Jake Farrell added a comment -

            removing the extern is not the correct approach due to systems that may have strlcpy already defined, and using _FORTIFY_SOURCE=0 in the cpp flags is a hacky workaround. instead i would like to see us use the __has_builtin macro to check and see if strlcpy is a builtin and if not then include the extern strlcopy

            #if !__has_builtin(strlcpy)
            extern size_t strlcpy(char *, const char *, size_t);
            #endif

            tested on mav and verifying this works on centos, ubuntu and and windows and then will commit

            jfarrell Jake Farrell added a comment - removing the extern is not the correct approach due to systems that may have strlcpy already defined, and using _FORTIFY_SOURCE=0 in the cpp flags is a hacky workaround. instead i would like to see us use the __has_builtin macro to check and see if strlcpy is a builtin and if not then include the extern strlcopy #if !__has_builtin(strlcpy) extern size_t strlcpy(char *, const char *, size_t); #endif tested on mav and verifying this works on centos, ubuntu and and windows and then will commit
            jfarrell Jake Farrell added a comment -

            commit 05f37f1

            jfarrell Jake Farrell added a comment - commit 05f37f1
            hudson Hudson added a comment -

            SUCCESS: Integrated in Thrift #986 (See https://builds.apache.org/job/Thrift/986/)
            THRIFT-2219: Thrift gem fails to build on OS X Mavericks (jfarrell: rev 05f37f1917bfa89d5862e4e45896bc6e28d8fa51)

            • lib/rb/ext/strlcpy.h
            hudson Hudson added a comment - SUCCESS: Integrated in Thrift #986 (See https://builds.apache.org/job/Thrift/986/ ) THRIFT-2219 : Thrift gem fails to build on OS X Mavericks (jfarrell: rev 05f37f1917bfa89d5862e4e45896bc6e28d8fa51) lib/rb/ext/strlcpy.h

            People

              jfarrell Jake Farrell
              chrismanderson Chris Anderson
              Votes:
              5 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: