Fix strict linking on MacOS

[jeroen]

Fixes#206.

This basically does the same as you already did on Windows here:

RcppParallel/src/Makevars.in

Lines 87 to 89 in 6f81716

	# Linker needs access to the tbb dll; otherwise you get errors such as:
	# "undefined reference to `tbb::task_scheduler_init::terminate()'"
	PKG_LIBS += -Ltbb/build/lib_release -ltbb -ltbbmalloc

And likewise in tbbLdFlags().

In my own fork I added CI for Mac, so you can see that it failed on MacOS before this fix, and then passed afterwards: https://github.com/jeroen/RcppParallel/commits/master/

[kevinushey]

LGTM, but I'd like to confirm a couple things before merge. With this approach, I see:

kevin@MBP-P2MQ:~/Library/R/arm64/4.3/library/RcppParallel/libs
$ otool -L RcppParallel.so
RcppParallel.so:
RcppParallel.so (compatibility version 0.0.0, current version 0.0.0)
@rpath/libtbb.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libtbbmalloc.dylib (compatibility version 0.0.0, current version 0.0.0)
/Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libomp.dylib (compatibility version 5.0.0, current version 5.0.0)
/Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libR.dylib (compatibility version 4.3.0, current version 4.3.1)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2202.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1600.157.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)

Could those @rpath entries cause a problem? I'm guessing this still works because we explicitly load the bundled TBB libraries before actually loading RcppParallel.so in our .onLoad(), so the library paths for libtbb and libtbbmalloc are effectively ignored?

Also, to confirm -- do those paths look the same / correct when cross-compiling?

[jeroen]

Update: actually it does work. I now added a -Wl,-rpath, to set @rpath to the correct location relative to the shared library. This makes the package load, even if you would comment out the dyn.load() code to manually find tbb.

I don't think there is any benefit right now because we are manually loading libtbb anyway but at least it is correct.

[kevinushey]

Awesome, thanks for taking the deeper look. LGTM!

[mtmorgan]

(maybe this is asking for a different feature, RcppParallel::RcppParallelLibs() including an appropriate -Wl,-rpath,..., rather than some shortcoming in the current PR)

On aarch64-apple-darwin23.2.0 I want to use tbb with cpp11, so I thought I could get a more-or-less robust solution w/ RcppParallel during linkage, but not actually requiring to import RcppParallel.

A test package is at https://github.com/mtmorgan/CppTest. It has the DESCRIPTION and src/Makevars file from https://rcppcore.github.io/RcppParallel/#r_packages; there is no imports(RcppParallel) in the NAMESPACE (since I'm not using RcppParallel in the package) and as a consequence RcppParallel is not loaded before CppTest. I have installed RcppParallel from github (remotes::install_github("RcppCore/RcppParallel")) When I try to install CppTest, i get

> install.packages("/tmp/CppTest", repos = NULL)
Installing package into '/private/tmp/filebaf33704bbd8'
(as 'lib' is unspecified)
* installing *source* package 'CppTest' ...
** using staged installation
** libs
using C++ compiler: 'Apple clang version 15.0.0 (clang-1500.1.0.2.5)'
using SDK: 'MacOSX14.2.sdk'
g++ -std=gnu++17 -I"/Users/ma38727/bin/R-devel/include" -DNDEBUG -I'/private/tmp/filebaf33704bbd8/cpp11/include' -I'/private/tmp/filebaf33704bbd8/RcppParallel/include' -I/opt/R/arm64/include -fPIC -g -O3 -c code.cpp -o code.o
g++ -std=gnu++17 -I"/Users/ma38727/bin/R-devel/include" -DNDEBUG -I'/private/tmp/filebaf33704bbd8/cpp11/include' -I'/private/tmp/filebaf33704bbd8/RcppParallel/include' -I/opt/R/arm64/include -fPIC -g -O3 -c cpp11.cpp -o cpp11.o
g++ -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -L/Users/ma38727/bin/R-devel/lib -L/opt/R/arm64/lib -o CppTest.so code.o cpp11.o -L/private/tmp/filebaf33704bbd8/RcppParallel/lib -ltbb -ltbbmalloc -L/Users/ma38727/bin/R-devel/lib -lR -Wl,-framework -Wl,CoreFoundation
installing to /private/tmp/filebaf33704bbd8/00LOCK-CppTest/00new/CppTest/libs
** R
** byte-compile and prepare package for lazy loading
** help
No man pages found in package 'CppTest' *** installing help indices
** building package indices
** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for 'CppTest' in dyn.load(file, DLLpath = DLLpath, ...):
unable to load shared object '/private/tmp/filebaf33704bbd8/00LOCK-CppTest/00new/CppTest/libs/CppTest.so':
dlopen(/private/tmp/filebaf33704bbd8/00LOCK-CppTest/00new/CppTest/libs/CppTest.so, 0x0006): Library not loaded: @rpath/libtbb.dylib
Referenced from: <04E31303-72AF-3E93-B02E-F4EE74E4BCDC> /private/tmp/filebaf33704bbd8/00LOCK-CppTest/00new/CppTest/libs/CppTest.so
Reason: tried: '/Users/ma38727/bin/R-devel/lib/libtbb.dylib' (no such file), '/Library/Java/JavaVirtualMachines/jdk-18.0.2.jdk/Contents/Home/lib/server/libtbb.dylib' (no such file)
Error: loading failed
Execution halted
ERROR: loading failed
* removing '/private/tmp/filebaf33704bbd8/CppTest'
* restoring previous '/private/tmp/filebaf33704bbd8/CppTest'
Warning message:
In install.packages("/tmp/CppTest", repos = NULL) :
installation of package '/tmp/CppTest' had non-zero exit status

(filebaf33704bbd8 is a temporary library path used to isolate the test build).

If I add import(RcppParallel) to the NAMESPACE, the installation (& use) succeeds.

This seems to be consistent with @kevinushey's concern that rpath is not resolved properly, and that the 'fix' works only because the tbb library is loaded before RcppParallel in RcppParallel's .onLoad.

CppTest/src main$ otool -L CppTest.so
CppTest.so:
CppTest.so (compatibility version 0.0.0, current version 0.0.0)
@rpath/libtbb.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/libtbbmalloc.dylib (compatibility version 0.0.0, current version 0.0.0)
libR.dylib (compatibility version 4.4.0, current version 4.4.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2202.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1600.157.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)

[kevinushey]

(maybe this is asking for a different feature, RcppParallel::RcppParallelLibs() including an appropriate -Wl,-rpath,..., rather than some shortcoming in the current PR)

I think this is right -- RcppParallelLibs() is incomplete right now, given the reliance on an rpath.