fj86 / PoissonBinomial

GNU General Public License v3.0
3 stars 0 forks source link

Please do not hardcode an arbitrary prefix for external libs #3

Open barracuda156 opened 2 months ago

barracuda156 commented 2 months ago

I see this happening when I build this in MacPorts:

/opt/local/bin/g++-mp-13 -std=gnu++17 -I"/opt/local/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include -I/usr/local/include   -I'/opt/local/Library/Frameworks/R.framework/Versions/4.4/Resources/library/Rcpp/include' -isystem/opt/local/include/LegacySupport -I/opt/local/include    -fPIC  -pipe -Os -arch ppc   -c RcppExports.cpp -o RcppExports.o
/opt/local/bin/g++-mp-13 -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/opt/local/Library/Frameworks/R.framework/Resources/lib -Wl,-headerpad_max_install_names -L/opt/local/lib -lMacportsLegacySupport -Wl,-rpath,/opt/local/lib/libgcc -arch ppc -o PoissonBinomial.so PoissonBinomial.o RcppExports.o -L/usr/local/lib -lfftw3 -F/opt/local/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation

This happens because Makevars hardcode /usr/local. This is a wrong behavior prone to producing breakages (consider that for some reason I actually had a second copy of libfftw3 in /usr/local). While MacPorts uses /opt/local, other package managers have different defaults. It should be left to a user or picked from environment.

fj86 commented 2 months ago

Thank you. I changed the Makevars file and will run the necessary tests (local, rhub, CRAN). An update should be ready within a week or so. Let me know if it solves your problem then.

barracuda156 commented 2 months ago

Thank you for addressing this. I think f0209d9a158fd097629d3ee7a7bdb829a89e0c67 should work fine (reasonable package manager is supposed to prefer its own prefix, so this should use correct headers and libs by default).

fj86 commented 2 months ago

Update 1.2.7 has been approved by CRAN and can be installed via R's install.packages() function, which (for now) builds it from source. Pre-compiled binaries should be ready within the next few days (you can check at https://cran.r-project.org/web/checks/check_results_PoissonBinomial.html). Please let me know if your problem is fixed.

barracuda156 commented 2 months ago

Update 1.2.7 has been approved by CRAN and can be installed via R's install.packages() function, which (for now) builds it from source. Pre-compiled binaries should be ready within the next few days (you can check at https://cran.r-project.org/web/checks/check_results_PoissonBinomial.html). Please let me know if your problem is fixed.

I will update the port once the new release is available either on your GH or CRAN. https://ports.macports.org/port/R-PoissonBinomial/details (For now I added a patch to fix prefix in Makevars, but this was indeed excessive, I just wanted to maintain code style. Now that can be dropped.)