Use rpath when compiling on macOS.

[hakonhagland]

This fixes issue #337 for DBD::mysql. When compiling on macOS the rpath must be explicitly set with linker option -Wl,-rpath or else the dynamic loader will refuse to load the library. Note that many workarounds exists like setting DYLD_LIBRARY_PATH or using install_name_tool. However, setting rpath for the generated executable is to be preferred and is the simplest solution.

[pali]

-rpath is linker specific option and -Wl is gcc/compiler specific option. Devel::CheckLib has already code to deal with rpaths, see:

p5-Devel-CheckLib/lib/Devel/CheckLib.pm

Line 416 in 593ac19

local $ENV{LD_RUN_PATH} = join(":", grep $_, @libpaths, $ENV{LD_RUN_PATH}) unless $^O eq 'MSWin32';

So if this code does not work for your platform, you should extend this code instead of adding ad-hoc hacks on other places.

I do not remember exact differences between -rpath, DYLD_LIBRARY_PATH and LD_RUN_PATH, so this needs to be investigated what option on which platform should be used. All these library paths are nightmare.

[hakonhagland]

Devel::CheckLib has already code to deal with rpaths,

@pali I have tested LD_RUN_PATH and it does not set rpath in the executable on macOS, you need to use -Wl,-rpath instead (this works for both clang and gcc), see also this issue.

[pali]

I see... But in this case Devel::CheckLib should not set $ENV{LD_RUN_PATH} when $^O eq "darwin", right?

[hakonhagland]

But in this case Devel::CheckLib should not set $ENV{LD_RUN_PATH} when $^O eq "darwin", right?

@pali Yes right, it is not needed but it does no harm to set it either I think. I did a quick test and commented out line 416 and it had no affect for perl Makefile.PL for DBD::mysql at least..

[pali]

Ok! I would suggest to not set additional ENV variables (or options) which does not work. It could confuse people in future if there would be another different problem. So what about following change? I tried to simplify it to use current coding style...

--- CheckLib.pm	2021-11-03 18:27:05.532843767 +0100
+++ CheckLib.pm	2021-11-03 18:33:43.065846954 +0100
@@ -407,13 +407,14 @@ sub assert_lib {
                 (map { "-I$_" } @incpaths),
                 $cfile,
                 (map { "-L$_" } @libpaths),
+                ($^O eq 'darwin' ? (map { "-Wl,-rpath,$_" } @libpaths) : ()),
                 "-l$lib",
                 @$ld,
                 "-o", "$exefile",
             );
         }
         warn "# @sys_cmd\n" if $args{debug};
-        local $ENV{LD_RUN_PATH} = join(":", grep $_, @libpaths, $ENV{LD_RUN_PATH}) unless $^O eq 'MSWin32';
+        local $ENV{LD_RUN_PATH} = join(":", grep $_, @libpaths, $ENV{LD_RUN_PATH}) unless $^O eq 'MSWin32' or $^O eq 'darwin';
         local $ENV{PATH} = join(";", @libpaths).";".$ENV{PATH} if $^O eq 'MSWin32';
         my $rv = $args{debug} ? system(@sys_cmd) : _quiet_system(@sys_cmd);
         if ($rv != 0 || ! -f $exefile) {

[hakonhagland]

So what about following change?

@pali It looks good to me. I have updated this PR with the change you suggested. I tested it for both DBD::mysql and DBD::MariaDB and it worked as expected.

[pali]

Perfect, thanks for testing!

[mohawk2]

It's great that this got merged. @mattn There hasn't been a release of this module since 2019. Please could you release this, and also in a perfect world #34 ?

If you're busy, I'll be happy to help out if you add me as a collaborator on here, and @DrHyde adds me as co-maint on PAUSE (I'm ETJ).

[DrHyde]

I've given you co-maint, but do please give @mattn a chance to respond before forking and making a release :-)

[mohawk2]

I've given you co-maint, but do please give @mattn a chance to respond before forking and making a release :-)

Thank you! I certainly wasn't planning on rushing into forking :-)

@mattn Thoughts?

[DrHyde]

Also added @zmughal as co-maint