search

astamm / nloptr

nloptr provides an R interface to NLopt, a free/open-source library for nonlinear optimization providing a common interface to a number of different optimization routines which can handle nonlinear constraints and lower and upper bounds for the controls.
https://astamm.github.io/nloptr/
Other
106 stars 34 forks source link

How to address CRAN request to remove C++11 when it is not needed? #130

Closed aadler closed 1 year ago

aadler commented 1 year ago

Hi, @astamm and @eddelbuettel. I'm opening an issue here since there was a recent discussion on R-package-devel specifically about how to handle the CRAN update request to C++17 when C++11 is all that is needed, thread starting here. I think @wch decided to go with the autoconfig option. Is that omething we want to do here, or are we willing to take the risk that nloptr will not build for someone using a version of GCC older than 4.8.1? Thoughts?

eddelbuettel commented 1 year ago

The horse you are beating here hasn't moved in a while. :grinning:

I am removing CXX_STD it from my packages, and am not adding support for R 3.6. Or R 1.5. Or SPlus 4. If someone runs an old system, they can patch my software (or define CXX as ${CXX} -std=c++11). CRAN is right: "make it work at @HEAD". CRAN is wrong, I think, in implying everybody should move to C++17.

aadler commented 1 year ago

LOL. I'm just paranoid about making breaking changes :)

Thanks, Dirk!

aadler commented 1 year ago

CXX calls removed in dev branch; heaven have mercy upon us.

wch commented 1 year ago

@aadler Just as a note, it turned out that the setting of CXX_STD, conditional on R version, in the Makevars file doesn't work. R still gives the C++11 NOTE.

I found out that it detects the C++ version by simply grepping Makevars for CXX_STD, here: https://github.com/wch/r-source/blob/22453c94794b946bc46a48bd1c379d0f8f9c5f1b/src/library/tools/R/install.R#L2641-L2653

Then the NOTE is raised here: https://github.com/wch/r-source/blob/08851e73ae5662b5a54474576afab0f19ad60c7d/src/library/tools/R/check.R#L5799-L5804

Right now my plan is to simply remove the CXX_STD from Makevars and the SystemRequirements: C++11 from DESCRIPTION. My understanding is that this will cause the package to no longer build on R 3.5, but as of R 3.6.2, C++11 became the default, so it should build on that version of R or higher.

eddelbuettel commented 1 year ago

Agreed! @kevinushey also already removed it in RcppParallel in the skeleton used to create packages, I did the same for RcppArmadillo and plan to carry the CXX_STD=CXX11 removal across my other packages as e.g. done here for digest.

Users stuck on old machines will have to locally modify e.g. their CXX in ~/.R/Makevars if they want to enable C++11 and their R version is too old to do it for them. Then again, R releases yearly and R 4.0.0 was a few moons ago.