nlmixr2 / rxode2parse

1 stars 0 forks source link

Undeclared linking to RcppParallel #94

Closed mfansler closed 1 month ago

mfansler commented 3 months ago

Building this package on Conda Forge, we detect that it links to RcppParallel. Perhaps this should be explicitly declared (LinkingTo and Imports)?

Pertinent conda-build linking analysis from Conda Forge build:

   INFO: sysroot: '/home/conda/feedstock_root/build_artifacts/r-rxode2parse_1717169502458/_build_env/x86_64-conda-linux-gnu/sysroot/' files: '['usr/share/zoneinfo/zone.tab', 'usr/share/zoneinfo/tzdata.zi', 'usr/share/zoneinfo/right/Zulu', 'usr/share/zoneinfo/right/WET']'
  ERROR (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO lib/R/library/RcppParallel/lib/libtbb.so.2 found in ['conda-forge/linux-64::r-rcppparallel==5.1.6=r43ha503ecb_1']
  ERROR (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): .. but ['conda-forge/linux-64::r-rcppparallel==5.1.6=r43ha503ecb_1'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO lib/R/lib/libR.so found in conda-forge/linux-64::r-base==4.3.3=hf0d99cb_1
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO lib/libstdc++.so.6 found in conda-forge/linux-64::libstdcxx-ng==13.2.0=hc0a3c3a_7
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libm.so.6 found in CDT/compiler package conda-forge/noarch::sysroot_linux-64==2.12=he073ed8_17
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO lib/libgcc_s.so.1 found in conda-forge/linux-64::libgcc-ng==13.2.0=h77fa898_7
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libpthread.so.0 found in CDT/compiler package conda-forge/noarch::sysroot_linux-64==2.12=he073ed8_17
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libc.so.6 found in CDT/compiler package conda-forge/noarch::sysroot_linux-64==2.12=he073ed8_17
   INFO (r-rxode2parse,lib/R/library/rxode2parse/libs/rxode2parse.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/ld-linux-x86-64.so.2 found in CDT/compiler package conda-forge/noarch::sysroot_linux-64==2.12=he073ed8_17
mattfidler commented 3 months ago

This package does not use RcppParallel but Stan Math Headers. Stan Math Headers took on the RcppParallel and likely needs it to compile.

It could be a reverse linking to...

mfansler commented 3 months ago

Okay, makes sense. To double-check it wasn't just us, I did also confirm the binaries on CRAN are also similarly linking:

otool -L rxode2parse.so
rxode2parse.so:
    rxode2parse.so (compatibility version 0.0.0, current version 0.0.0)
    /Library/Frameworks/R.framework/Versions/4.4-x86_64/Resources/lib/libRlapack.dylib (compatibility version 4.4.0, current version 4.4.0)
    /Library/Frameworks/R.framework/Versions/4.4-x86_64/Resources/lib/libRblas.dylib (compatibility version 0.0.0, current version 0.0.0)
    /Library/Frameworks/R.framework/Versions/4.4-x86_64/Resources/lib/libgfortran.5.dylib (compatibility version 6.0.0, current version 6.0.0)
    /Library/Frameworks/R.framework/Versions/4.4-x86_64/Resources/lib/libquadmath.0.dylib (compatibility version 1.0.0, current version 1.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.4-x86_64/Resources/lib/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 1775.118.101)
    /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 905.6.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)

I'm mainly concerned that under the current declarations, installing a precompiled binary will not install RcppParallel since StanHeaders is only a LinkingTo declaration. Hence, users could end up encountering cryptic runtime errors, where they are told they need libtbb but don't realize this is expected to be sourced through RcppParallel.

On the Conda Forge side, we can add directives to export RcppParallel as a runtime dependency for other packages whenever StanHeaders is used at build time. For CRAN packaging, I don't think there is equivalent auto-exporting - hence why I expect this package probably should declare it under Imports.

mattfidler commented 3 months ago

I can do this on the next release. However it may take some time to get to cran

mattfidler commented 1 month ago

This package will eventually be removed from CRAN (at their request) and combined with rxode2. That version has RcppParallel in the LinkingTo field.

mfansler commented 1 month ago

Thanks Matthew - and for the heads up on repackaging!