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

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.1
    • Fix Version/s: 0.9.2
    • Component/s: Ruby - Library
    • Labels:
      None
    • Environment:

      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.

        Issue Links

          Activity

          Hide
          ccutrer Cody Cutrer added a comment -

          note that Mavericks is now GM, so essentially released

          Show
          ccutrer Cody Cutrer added a comment - note that Mavericks is now GM, so essentially released
          Hide
          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.

          Show
          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.
          Hide
          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.

          Show
          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.
          Hide
          mroth Michael Roth added a comment -

          Ben Nham's hack works!

          Show
          mroth Michael Roth added a comment - Ben Nham's hack works!
          Hide
          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 )

          Show
          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 )
          Hide
          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.

          Show
          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.
          Hide
          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

          Show
          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
          Hide
          jfarrell Jake Farrell added a comment -

          commit 05f37f1

          Show
          jfarrell Jake Farrell added a comment - commit 05f37f1
          Hide
          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
          Show
          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

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development