minimal patch for build against nlopt 2.9.x

[jaganmn]

Fixes #173 and fixes #175.

[jaganmn]

I've checked this PR against all of nlopt 2.7.0, 2.7.1, 2.8.0, 2.9.0, 2.9.1, and 2.10.0, as well as without a system nlopt, when nlopt 2.7.1 is built from the internal source tarball. Two tests in inst/tinytest/test-nloptr.R fail when checking against nlopt >= 2.9.0. I will create a new issue, as the problem seems distinct from the build system issue.

[jaganmn]

A corollary of this PR is that nlopt.h is no longer copied into inst/include when nlopt is built from the internal source tarball. nloptrAPI.h still does

#include <nlopt.h>

so packages with LinkingTo: nloptr that compile nloptrAPI.h technically need SystemRequirements: nlopt. In principle, those reverse LinkingTo always needed SystemRequirements: nlopt, because if nloptr were built against a system nlopt, then nloptrAPI.h would not compile without the continued availability of the system nlopt.

Adding just the necessary typedefs to the top of nloptrAPI.h is a possibility, but then you need to be somewhat worried about ABI breakage. In any case, you should definitely run reverse dependency checks before submitting to CRAN.

[eddelbuettel]

Any idea about the GH failures? It "works for me" with the standard libnlopt in Ubuntu, ie 2.7.1.

[jaganmn]

Not sure and hard to say without the configure/install logs. Maybe the failure arises only if there is no system nlopt, though I tested that case locally (macOS). What happens if you disable pkg-config, forcing a local build of nlopt?

The nloptrAPI.h situation needs some attention, too, since it seems like there really are reverse LinkingTo (such as RcppNLoptExample) operating on the false promise that the include directory contains a static copy of nlopt.h. I can revert that part of the PR (the deleted line in src/scripts/nlopt_cleanup.sh), I guess, with the understanding that the situation will continue to be broken, albeit in ways that people out there are not detecting ...

[eddelbuettel]

Being a personal fan of (much) simpler CI settings, I copied my own standard (generally 'constant' but here needed one apt call for libnlopt-dev; we could make it a matrix and have it compile locally too) and tried it. It needed one test to be commented out (commit here, maybe @aadler wants to look at that) after which it succeeded with 'green' CI. That is based on your PR branch, of course.

How packages use other packages at the compiled level is actually surprisingly tricky as there are in fact multiple approaches. I recently created / overhauled the (at is core very simple) PRNG package zigg. There, I added four different embedded examples packages using zigg from compiled package code. You may find these interested. Because contrary to your gut feel, RcppNLoptExample still builds and test when I install your PR here. The C++ variant in RcppArmadilloNLoprExample fails as we no longer install nlopt.hpp. It works with the current CRAN nloptr if a LinkingTo is added as your surmise. I may need to revisit those examples; that is a little orthogonal to getting nloptr back into shape.

[jaganmn]

Thanks for the additional checks.

Locally, for me, even RcppNLoptExample fails to build (with error 'nlopt.h' file not found) when I remove the system nlopt, install nloptr from this PR, then attempt to install RcppNLoptExample.

[eddelbuettel]

Yes -- I am not certain the example works without a system nlopt. This arose in part out of two users having persistent issue with this so I wanted a worked example which worked for them too at the time.

Consistently supplying an external library either as system or as baked-in is twice as hard. It get easier with explicit dependencies.

[aadler]

It needed one test to be commented out (commit here, maybe @aadler wants to look at that)

Yes, that is one which has randomness built into it and so is a bit hard to get to converge to the same value at same precision across Windows/Mac/Linux flavors. Fine with commenting it out for now and once the big issue of the package robustness to nlopt is solved, I will go back and review all the tests and see what I can do to make them more robust. Thank you!

[astamm]

In any case, you should definitely run reverse dependency checks before submitting to CRAN.

Thanks @jaganmn. Yes I will do that which I always do before submitting to CRAN anyway.

[astamm]

We can add nlopt in the SysemRequirements field to be consistent. We could even specify the version to be 2.10 and, as @aadler suggested, fail install if a system nlopt other than latest release is detected, prompting the user to update.

[eddelbuettel]

Please do not do that forced update. Version 2.7.1 as present in Debian and Ubuntu and hence on a lot of systems is perfectly fine.

[aadler]

I would defer to @eddelbuettel here; he has significant experience in maintaining packages with system library requirements (RcppArmadillo, etc.). So long as @jaganmn patch allows any version of nlopt to be used, we should be as unobtrusive as possible in the install. I mean, the nlopt documents themselves claim 2.7.1 is the latest in some places, which calls their maintenance into askance, but we can't do much being downstream.

[astamm]

The nloptrAPI.h situation needs some attention, too, since it seems like there really are reverse LinkingTo (such as RcppNLoptExample) operating on the false promise that the include directory contains a static copy of nlopt.h. I can revert that part of the PR (the deleted line in src/scripts/nlopt_cleanup.sh), I guess, with the understanding that the situation will continue to be broken, albeit in ways that people out there are not detecting ...

I'll update you all on the result of the reverse dependency checks to see if the current fix fails other reverse LinkingTo deps.

[eddelbuettel]

I can get it to pass in another local branch of this PR branch under both system nlopt and the included nlopt if exclude just one line in test file ... as well as the final 16 of 48 tests in a larger test file.

See https://github.com/eddelbuettel/nloptr/actions/runs/13634481074

[astamm]

Great. Not sure what's wrong with r-lib action. Some buffer size limit apparently.

I suspect that with the addition in my first commit here, all tests pass. So you can unskip the one you had to skip. I am in favour of the simple CI you used. Do you want to merge your local branch? Or do you prefer I simply reuse your .yaml files?

[astamm]

Oh no, I got it wrong. You meant you skip the rest of the testing suite in that file to avoid the buffer size limit issue.

[eddelbuettel]

Correct. tinytest logs to stdout clear show that 32 (of the 48) tests pass. More finegrained analysis may reveal we only need to skip 1, 2, ... of those 16.

That, plus one commenting out of one other line does it. I am not a fan of r-lib actions which I find to opaque and harder to debug but I don't think they are to blame here. There appears to be a change vector from nlopt later than 2.7.* which we can investigate in due course.

My recommendation is still to move ahead, get the package cleaned and onto CRAN and then dive into details.

[astamm]

I will do that. Since I investigated a bit, it seems only the 33rd is at fault. See https://github.com/r-hub2/demented-alkalisable-chameleon-nloptr/actions/runs/13636482812.

Not sure yet what's wrong with this test but it seems we can still perform all the other 15.

[eddelbuettel]

I get it to by commenting out the next eight. I first tried the next four, that did not work. So 'NLOPT_LN_NEWUOA_BOUND' and 'NLOPT_GN_ESCH' are skipped.

[aadler]

Fine with me. Once we have the program working, we can work on bringing the coverage back to 90+%. Also, given that nlopt 2.9 doesn't have the NLOPT_LD_LBFGS_NOCEDAL at all, it makes sense why that test would fail on 2.9. Since I expect we need to support 2.9 AND 2.10 for a while, that test should probably be removed.

[eddelbuettel]