Skip to content

changes to pick up correct ruby version#2

Closed
krgn wants to merge 6 commits intob4winckler:masterfrom
krgn:master
Closed

changes to pick up correct ruby version#2
krgn wants to merge 6 commits intob4winckler:masterfrom
krgn:master

Conversation

@krgn
Copy link
Contributor

@krgn krgn commented Oct 31, 2010

hi, as per the discussion here http://vim.1045645.n5.nabble.com/MacVim-Ruby-1-9-1-td1220105.html I re-created the changes made to src/configure and src/configure.in to make it pick up the right ruby version. I was then able to do ./configure --enable-rubyinterp --with-ruby=which ruby and link against 1.9.2 installed with rvm. thanks to the author of the initial patch!

@b4winckler
Copy link
Owner

There is another problem with the original patch. I sent the following to the original author (Michael Shapiro) but he never got back to me. Can you fix it?

At the top of the patch you set RUBY_PATH regardless whether
--with-ruby-command is passed to configure or not (as far as I can
tell). Later on you have a check like "if test "X$RUBY_PATH" != "X";
then" so isn't this test always going to pass? That is not so good:
when I build a universal binary I need this test to fail so that
RUBY_LIBS is set to "-framework Ruby" and RUBY_CFLAGS and librubyarg
need to be empty.

@krgn
Copy link
Contributor Author

krgn commented Oct 31, 2010

Hm true, thats no good. I added a check now whether system ruby is used (either by specifying --with-ruby-command=/usr/bin/ruby explicitly or for the case when no other version of ruby is installed) which leads it to link against the system framework. Could you double check this though? I did test it and I think it does the trick but its good to see if it works right away for you.

cheers,
karsten

@b4winckler
Copy link
Owner

I don't think this works. Around line 6381 there is a test to see if RUBY_PATH is empty, but it is always set at the beginning of the first patch. I haven't got the time to check this carefully right now, so could you please look into it?

The way this used to work (before this patch) was that RUBY_CMD was only set if you passed a flag to configure. Maybe something similar should be done with RUBY_PATH, i.e. only set it if --with_ruby_command is used?

@krgn
Copy link
Contributor Author

krgn commented Oct 31, 2010

What I thought is the easiest for now is to just switch around the check so that RUBY_PATH is checked first for system ruby (RUBY_PATH == /usr/bin), then goes on to using whatever else was specified. I agree this is not as clean as it could be, but like my predecessor I'm no expert at autoconf.

But anyways, there it is I think:

./configure --enable-rubyinterp --with-ruby-command=/usr/bin/ruby

during linking:

gcc -L. -L/usr/local/lib -o Vim objects/buffer.o objects/blowfish.o objects/charset.o objects/diff.o objects/digraph.o objects/edit.o objects/eval.o objects/ex_cmds.o objects/ex_cmds2.o objects/ex_docmd.o objects/ex_eval.o objects/ex_getln.o objects/fileio.o objects/fold.o objects/getchar.o objects/hardcopy.o objects/hashtab.o objects/if_cscope.o objects/if_xcmdsrv.o objects/main.o objects/mark.o objects/memfile.o objects/memline.o objects/menu.o objects/message.o objects/misc1.o objects/misc2.o objects/move.o objects/mbyte.o objects/normal.o objects/ops.o objects/option.o objects/os_unix.o objects/pathdef.o objects/popupmnu.o objects/quickfix.o objects/regexp.o objects/screen.o objects/search.o objects/sha256.o objects/spell.o objects/syntax.o objects/tag.o objects/term.o objects/ui.o objects/undo.o objects/window.o objects/gui.o objects/gui_beval.o objects/pty.o objects/gui_macvim.o objects/MMBackend.o objects/MacVim.o objects/if_ruby.o objects/os_macosx.o objects/os_mac_conv.o objects/netbeans.o objects/version.o -framework Cocoa -framework Carbon -lm -lncurses -liconv -framework Cocoa -framework Ruby
link.sh: OK, linking works, let's try omitting a few libraries.
link.sh: See auto/link.log for details.
link.sh: Trying to omit the iconv library...
link.sh: Vim DOES need the iconv library.
link.sh: Using unmodified link command
gcc -L. -L/usr/local/lib -o Vim objects/buffer.o objects/blowfish.o objects/charset.o objects/diff.o objects/digraph.o objects/edit.o objects/eval.o objects/ex_cmds.o objects/ex_cmds2.o objects/ex_docmd.o objects/ex_eval.o objects/ex_getln.o objects/fileio.o objects/fold.o objects/getchar.o objects/hardcopy.o objects/hashtab.o objects/if_cscope.o objects/if_xcmdsrv.o objects/main.o objects/mark.o objects/memfile.o objects/memline.o objects/menu.o objects/message.o objects/misc1.o objects/misc2.o objects/move.o objects/mbyte.o objects/normal.o objects/ops.o objects/option.o objects/os_unix.o objects/pathdef.o objects/popupmnu.o objects/quickfix.o objects/regexp.o objects/screen.o objects/search.o objects/sha256.o objects/spell.o objects/syntax.o objects/tag.o objects/term.o objects/ui.o objects/undo.o objects/window.o objects/gui.o objects/gui_beval.o objects/pty.o objects/gui_macvim.o objects/MMBackend.o objects/MacVim.o objects/if_ruby.o objects/os_macosx.o objects/os_mac_conv.o objects/netbeans.o objects/version.o -framework Cocoa -framework Carbon -lm -lncurses -liconv -framework Cocoa -framework Ruby
link.sh: Linked OK

I tried the app afterwards and it behaved like I expected (didn't work, because CommandT C extensions will crash it if built with a different ruby version, and, fwiw, it would display 1.8.7 as ruby-version). I think this covers all the cases, but let me know if not!

@b4winckler
Copy link
Owner

Hmmm...ok, perhaps this does make it work, but I think it is messy. What happens if Apple decides to move the default ruby binary?

If you are able, please try to make a patch like I suggested previously. Otherwise I'll have to take a look myself (I'm not big on autotools myself which is why I was hoping I didn't have to do this).

@krgn
Copy link
Contributor Author

krgn commented Nov 1, 2010

I think I found the problem in the original configure file. The reason why it doens't in the end pick up the ruby thats in the path is because of this test:

     librubyarg=`$vi_cv_path_ruby -r rbconfig -e 'print Config.expand(Config::CONFIG["LIBRUBYARG"])'`

returns -lruby.1.9.1

     if test -f "$rubyhdrdir/$librubyarg"; then
        librubyarg="$rubyhdrdir/$librubyarg"
      else

where configure is trying to find a file of the name '-lruby.1.9.1' in the header dir of the ruby installation which never exists, which is why it then switches to checking for the framework:

     if test -d "/System/Library/Frameworks/Ruby.framework"; then
          RUBY_LIBS="-framework Ruby"
          RUBY_CFLAGS=
          librubyarg=

which is always true in the case of OSX dev setups resulting in it linking against this regardless. The fix is obvious. This test:

      if test -f "$rubyhdrdir/$librubyarg"; then
        librubyarg="$rubyhdrdir/$librubyarg"
      else

should be changed to do what I think its meant to do as far as I understand, check for the presence of the libs (btw, should it check for static libraries *.a or the .dylib ones?). But the problem with what it will link to remains, as even in the case of the system ruby its possible to link using -lruby thus not producing a UB, right? So somehow we have to add a check for a flag or value (I'd propose 'system' value or similar) that will make it use the framework, specifically, or check whether

    ruby -r rbconfig -e 'p Config.expand(Config::CONFIG["LIBRUBYARG"])'

returns -lruby so that whenever the system ruby is used it will always build with -framework. You decide, I fix it :)

@b4winckler
Copy link
Owner

I think I'm following you but in case my answer makes little sense, feel free to explain it to me.

As for .dylib or .a, I'd say go for .dylib (everything else does).

As for whether to use -framework or pick up the values from librubyarg, I have two suggestions:

  1. Use -framework unless the --with-ruby switch was passed to configure
  2. Use -framework if MACSDK is non-empty (I prefer 1. since this would save me from having to pass --with-macsdk to build a universal binary), like this
    if test -n "$MACSDK"; then
    RUBY_LIBS="-framework Ruby"
    RUBY_CFLAGS=
    librubyarg=
    fi

@krgn
Copy link
Contributor Author

krgn commented Nov 2, 2010

hey, I have checked in a working version of the configure file. It sticks to case 1. you made, i.e. only picks up custom ruby when specifically asked for. Both cases built and worked fine. I do still need to modify the configure.in to reflect the changes though!

@krgn
Copy link
Contributor Author

krgn commented Nov 3, 2010

hey, do you want to double check this, I've fixed the autoconf script so it'll generate configure properly.

one thing I found was that in the configure script in your git repo a particular part is missing which was not commented out in configure.in, the part that checks whether --enable-ruby=dynamic was set and enables this path through the code. for now I have commented out that part in configure.in so it doesn't find its way into configure when being re-generated under the assumption that its intentionally missing.

greetings

@b4winckler
Copy link
Owner

I have to take a closer look, but I don't think you should hard code paths like RUBY_CMD="/usr/bin/ruby". The configure script is supposed to work on all platforms Vim supports.

Another thing that I just remember is that in if_ruby.c I always do #include <Ruby/ruby.h> as if -framework was used. This should be #include <ruby.h> unless linking against the framework. Don't you get errors when not linking against the framework? If not, I think it is picking up the wrong headers. I looked at this some time ago and there was a problem with this in that vim.h is only included after ruby.h include so any define set in the configure script will not be set by the time ruby.h is included (which makes it hard to decide whether to include Ruby/ruby.h or ruby.h.

Phew. Anyway, I have to look at your patch later.

@krgn
Copy link
Contributor Author

krgn commented Nov 3, 2010

OK, its no problem to remove it and set it to 'ruby' only with the side effect that if one's ruby in $PATH != /usr/bin/ruby the program will be built with ruby headers different from the libs it will be linked against with -framework, and this is default behavior. The cross-platform argument is really a problem though, I just didn't think this would become a problem since its for macvim :) Any advice welcome!
Regarding the other issue: I havn't seem any problems so far but the there is a possibility it used the wrong headers before, though I'm pretty sure now it includes the righth headers:

CC="gcc -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_MACVIM -Wall -Wno-unknown-pragmas -pipe  -DMACOS_X_UNIX -no-cpp-precomp     -I/Users/karsten/.rvm/rubies/ruby-1.9.2-p0/include/ruby-1.9.1 -I/Users/karsten/.rvm/rubies/ruby-1.9.2-p0/include/ruby-1.9.1/x86_64-darwin10.4.0 -DRUBY_VERSION=19 " srcdir=. sh ./osdef.sh

and

  gcc   -L. -L/usr/local/lib -L. -L/usr/local/lib  -L/usr/local/lib     -o Vim objects/buffer.o objects/blowfish.o objects/charset.o objects/diff.o objects/digraph.o objects/edit.o objects/eval.o objects/ex_cmds.o objects/ex_cmds2.o objects/ex_docmd.o objects/ex_eval.o objects/ex_getln.o objects/fileio.o objects/fold.o objects/getchar.o objects/hardcopy.o objects/hashtab.o  objects/if_cscope.o objects/if_xcmdsrv.o objects/main.o objects/mark.o objects/memfile.o objects/memline.o objects/menu.o objects/message.o objects/misc1.o objects/misc2.o objects/move.o objects/mbyte.o objects/normal.o objects/ops.o objects/option.o objects/os_unix.o objects/pathdef.o objects/popupmnu.o objects/quickfix.o objects/regexp.o objects/screen.o objects/search.o objects/sha256.o objects/spell.o objects/syntax.o  objects/tag.o objects/term.o objects/ui.o objects/undo.o objects/window.o objects/gui.o objects/gui_beval.o objects/pty.o objects/gui_macvim.o objects/MMBackend.o objects/MacVim.o       objects/if_ruby.o objects/os_macosx.o objects/os_mac_conv.o  objects/netbeans.o  objects/version.o -framework Cocoa -framework Carbon      -lm -lncurses  -liconv -framework Cocoa        -lruby.1.9.1 -L/Users/karsten/.rvm/rubies/ruby-1.9.2-p0/lib  

link.sh: Linked OK

Both suggest that its doing the right thing. Hmm... let me know if you have an idea!

@b4winckler
Copy link
Owner

The -I flags should make no difference for picking up the right headers since custom Ruby versions usually aren't packaged as frameworks, which means that there is no file called Ruby/ruby.h in the include files path (the "Ruby" folder simply isn't there). I'm really confused as to why it works at all to use a custom Ruby to be honest.

I'm not sure if I'm misunderstanding your comment about ruby != /usr/bin/ruby, but if you do specify the -framework flag then the linker and compiler will use the headers/libraries from one and the same framework. However, when you run ruby from inside Vim it will probably use the "wrong" ruby binary and get some weird error inside Vim (I'm guessing).

I don't have any more comments at this point in time. Note that I still don't fully understand the issues you are trying to overcome here. Maybe explaining the situation on the vim_dev Google Group and asking for help there may work (don't hold your breath though).

@krgn
Copy link
Contributor Author

krgn commented Nov 4, 2010

The problem is relatively simple: for example, the location of the headers is set using inbuilt ruby config mechanisms. It gets the location of the header files like this:

rubyhdrdir=`$vi_cv_path_ruby -r mkmf -e 'print Config::CONFIG["rubyhdrdir"] || Config::CONFIG["archdir"] || $hdrdir' 2>/dev/null`

So, when you get the case that the user doesn't specify a particular ruby command explicitly with the --with-ruby-command flag the script would default to just 'ruby', i.e. whatever is in your $PATH, but -- and this is the main problem with this approach -- use the hardcoded -framework switch for linking. The danger is obvious: if you do in fact have a custom ruby installed through ports or homebrew but don't actually know that you have to explicitly specify the ruby command with --with-ruby-command in order to link correctly it will result in using headers obtained like above, but link to the wrong libs.
This is why I personally favor the default using whatever is in the path, unless specified differently, and introduce a flag to --with-ruby-command that will specifically select the -framework switch. This is cleanest solution if you really want to make sure that the configure script stays a) largely platform/system agnostic, b) will produce reliable and consistent results (i.e. include the right api and libs) and c) will be somewhat user friendly by providing sane defaults. My 2¢.

@b4winckler
Copy link
Owner

Thanks for the explanation.

Can you make it work the way you suggest and make it so that -framework is used if the MACSDK flag is set? I can certainly jump through that small hoop in order to make building easier for everybody else.

Actually, can you just make a minimal patch that does things your way and cut the -framework flag altogether? This way we may be able to get the patch accepted into the upstream Vim source code and I can just add the MACSDK part myself if I really need it (I'm not sure that I actually do).

@krgn
Copy link
Contributor Author

krgn commented Nov 4, 2010

ok cool, will do. btw: do you actually just take the autoconf scripts the way they ship from upstream or tweak them yourself? I'm asking because there is stuff in src/configure.in that is enabled which is not in src/configure (which suggests that src/configure was edited directly instead of generated using autoconf).

@b4winckler
Copy link
Owner

I pull from core Vim, but there are changes in MacVim to configure.in. The configure script itself is sometimes regenerated in upstream and I don't regenerate it myself every time. Maybe that explains the discrepancy? You can always write your patch against the configure.in script that is in upstream. That will make it easier to get it accepted in the upstream source as well.

@krgn
Copy link
Contributor Author

krgn commented Nov 5, 2010

hi, I've made a patch agains upstream vim. would you like to send it to the dev (i'm not on that one)? you can find it here:

https://github.com/krgn/patches

@b4winckler
Copy link
Owner

It would be great if you forked b4winckler/vim.git (this is MacVim's upstream) and created a branch with your patch on it. Then I'll take a look at it and afterwards we can send it on to vim_dev. How does that sound?

@krgn
Copy link
Contributor Author

krgn commented Nov 5, 2010

ok thats cool, see pull request

@b4winckler
Copy link
Owner

A quick question: will the following patch (to MacVim's configure.in) work or not? (I don't have a custom Ruby installed to test with.)

    diff --git a/src/configure.in b/src/configure.in
    index 46e3e9a..7086a70 100644
    --- a/src/configure.in
    +++ b/src/configure.in
    @@ -1365,13 +1365,7 @@ if test "$enable_rubyinterp" = "yes" -o "$enable_rubyinterp" = "dynamic"; then
              librubyarg="$rubyhdrdir/$librubyarg"
            else
              rubylibdir=`$vi_cv_path_ruby -r rbconfig -e 'print Config.expand(Config::CONFIG[["libdir"]])'`
    -          if test -d "/System/Library/Frameworks/Ruby.framework"; then
    -            dnl On Mac OS X it is safer to just use the -framework flag
    -            RUBY_LIBS="-framework Ruby"
    -            dnl Don't include the -I flag when -framework is set
    -            RUBY_CFLAGS=
    -            librubyarg=
    -          elif test -f "$rubylibdir/$librubyarg"; then
    +          if test -f "$rubylibdir/$librubyarg"; then
                librubyarg="$rubylibdir/$librubyarg"
              elif test "$librubyarg" = "libruby.a"; then
                dnl required on Mac OS 10.3 where libruby.a doesn't exist

@krgn
Copy link
Contributor Author

krgn commented Nov 5, 2010

no, because libruby arg is a string like this -lruby.1.9.1 to tell the linker to link against this lib (though my understanding of this process is somewhat superficial), so the the test is checking for a file in $rubylibdir with that name, which does not exist. So in order to build it, you need to set RUBY_LIBS="-L$rubylibdir $librubyarg" whcih expands to "-L/path/to/lib/libruby.a -lruby1.9.1" or somethign like that.

@rosstimson
Copy link

I can't comment on the coding/elegance of this solution but I was having problems compiling MacVim against Ruby-1.9.2p136 (under RVM). This branch/patch got me up and running no problem. Many thanks krgn.

@b4winckler
Copy link
Owner

I submitted this patch to vim_dev for inclusion in the Vim mainline but I never received a reply. Instead of waiting for this patch to be included in mainline I decided to include it in MacVim for now.

Thanks for the patch.

@krgn
Copy link
Contributor Author

krgn commented Feb 16, 2011

thank you, both, for submitting and eventually including it!

splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
splhack referenced this pull request in splhack/macvim Dec 5, 2014
This pull request was closed.
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.

3 participants

Comments