[vim/vim] fix dynamic Ruby linker arguments (PR #20558)
Vladimír Marek
Make configure keep Ruby linker arguments when Ruby support is built with --enable-rubyinterp=dynamic. The dynamic Ruby branch clears RUBY_LIBS before setting DYNAMIC_RUBY, so add librubyarg after that branch instead of before it.
Related: #17906
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20558
Commit Summary
- cb1dc1f fix dynamic Ruby linker arguments
File Changes
(2 files)
- M src/auto/configure (8)
- M src/configure.ac (7)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20558)thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Johannes Schindelin
dscho left a comment (vim/vim#20558)This seems to completely break the idea of a dynamic binding to Ruby: With this, the vim.exe in MSYS2 (which formerly loaded the Ruby DLL only on demand, at runtime), no longer starts:
$ vim any-file
D:/msys64/usr/bin/vim.exe: error while loading shared libraries: msys-ruby340.dll: cannot open shared object file: No such file or directory
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20558)Oops sorry about that. Can you check if this patch fixes it?
diff --git a/src/configure.ac b/src/configure.ac index bac223698..710cb317f 100644 --- a/src/configure.ac +++ b/src/configure.ac @@ -2162,6 +2162,16 @@ if test "$enable_rubyinterp" = "yes" -o "$enable_rubyinterp" = "dynamic"; then AC_DEFINE(DYNAMIC_RUBY) RUBY_CFLAGS="-DDYNAMIC_RUBY_DLL=\\\"$libruby_soname\\\" $RUBY_CFLAGS" RUBY_LIBS= + dnl For dynamic Ruby keep only the library search path / runpath from + dnl librubyarg so the dlopen()ed library can be found at runtime, + dnl without adding -lruby and thus a hard dependency on the Ruby + dnl library, which would defeat dynamic loading. + for arg in $librubyarg; do + case "$arg" in + -L*|-R*|-Wl,-R*|-Wl,-rpath*) RUBY_LIBS="$RUBY_LIBS $arg" ;; + esac + done + librubyarg= fi if test "X$librubyarg" != "X"; then RUBY_LIBS="$librubyarg $RUBY_LIBS"
You'd need to run make autoconf afterwards.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Vladimír Marek
vlmarek left a comment (vim/vim#20558)So the link flags are basically split into two groups - one that allow you to record information to resulting binary in order to successfully call dlopen later (not sure how windows do that) and second are parameters which are necessary in order to link the binary to given library. By this logic -L does not have to be here but causes no harm.
Sorry about this and thank you for the correction.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20558)should be fixed now with 9.2.0681
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
ychin left a comment (vim/vim#20558)I am still not sure if this change was necessary to begin with. Is the point there just to add the -R flag to specify search path? To me, the whole point of dynamic linking is that you decouple Vim from the build environment and lets the user dynamically switch which dynamic library to use. Instead of baking in a hidden search path to the binary, I think a better solution is to simply bake in the full path of the dynamic lib. So basically the compilation step would hard code rubydll setting to be /usr/ruby/3.3/lib/amd64/libruby.3.3.0.so rather than setting it to libruby.3.3.0.so and hoping that the search path works out. This is also much more intuitive and obvious to a user which library is being used (and secure as well as you remove implied search orders).
One way to fix this is to make configure.ac smarter. For example, due to Python 3 being installed in odd locations on a Mac, the configure script has detection logic to find that (see #12189) and automatically hard codes the pythonthreedll setting to be /opt/homebrew/Frameworks/Python.framework/Versions/Current/Python.
Alternatively, we could provide a way for the builder to manually override the name/path of the rubydll setting. Currently this functionality exists for lua (vi_cv_dll_name_lua) / python (vi_cv_dll_name_python3) / etc, but not for Ruby. This is the most flexible option and Ruby seems to be the odd one out that doesn't support it (MacVim's build script actually added this downstream to make it possible to build a sensible binary).
But yes, currently this is breaking dynamic ruby builds on Mac as well.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
ychin left a comment (vim/vim#20558)I guess thinking more about it, it's fine… It feels a bit fragile as we are manually parsing these linker flags, but it mostly works.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Vladimír Marek
vlmarek left a comment (vim/vim#20558)I do not see clean simple solution. Linkflags are are abused to do something they are not generally supposed to do. I'm happy to work on it more or be told that this is solaris quirk which does not belong to vim. No hard feelings :) Thank you
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Johannes Schindelin
dscho left a comment (vim/vim#20558)should be fixed now with 9.2.0681
Thank you @chrisbra! I'll try to remember testing this without the revert in MSYS2, but I will only have time after the Git for Windows v2.55.0 release craziness (i.e. probably some time next week).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.