Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: build against nlopt 2.7.1 #92

Merged
merged 1 commit into from
Jan 17, 2022
Merged

Windows: build against nlopt 2.7.1 #92

merged 1 commit into from
Jan 17, 2022

Conversation

jeroen
Copy link
Contributor

@jeroen jeroen commented Jan 17, 2022

Tested to work + CI on R 4.1, 4.1, 4.2

@astamm
Copy link
Owner

astamm commented Jan 17, 2022

Thank you so much @jeroen. Running the CI checks now.
If I understand correctly, I think we should maybe add a step in the tools/winlibs.R script to copy nlopt headers into inst/include so that we can also directly call the C and C++ API of nlopt in other packages as well (as I do for example).
Does that make sense?

@eddelbuettel
Copy link
Contributor

And/or call another script or copy step. The current tools/winlibs.R looks like all the others for the 80-some libraries, it may be beneficial to keep it that way.

@astamm astamm merged commit 397e765 into astamm:master Jan 17, 2022
@eddelbuettel
Copy link
Contributor

(And while you are doing some polishing before another upload, the Description: text about unconditional cmake and required cmake installation is no longer true. Maybe state as 'needed when building from source' and note where? Maybe 'macOS and Linux (unless NLopt installation found)' ?

@astamm
Copy link
Owner

astamm commented Jan 17, 2022

I was just thinking about that indeed. The SystemRequirements field is text-free, right? So I guess we could provide details in parentheses there. But if we leave cmake in SystemRequirements, it is not technically true either...

@eddelbuettel
Copy link
Contributor

Yes it is -- see for example what I did for package tiledb (where we either do installed or download or build, it even more messy). Hard to get right or perfect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants