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

update makevars for CGAL 6.0.1 #56

Merged
merged 4 commits into from
Nov 27, 2024
Merged

update makevars for CGAL 6.0.1 #56

merged 4 commits into from
Nov 27, 2024

Conversation

ericdunipace
Copy link
Contributor

Hi, the new version of the CGAL headers has a new flag to disable GMP. I have a suggested pull request so that I can push the new header files to CRAN. An alternative is to link GMP in your makevars, which may be undesirable.

@m-jahn
Copy link
Owner

m-jahn commented Nov 25, 2024

Dear Eric, thanks for the heads-up and the work you put in RcppCGAL maintenance.
I will try to test and merge your PR. What consequences will this flag have? There's already a NO-GMP flag but now there are two somewhat equivalent flags? Regarding the time line of package submission: Will my package fail on CRAN as soon as you submit your new release with the breaking change?

@m-jahn
Copy link
Owner

m-jahn commented Nov 25, 2024

I have checked your commit against the current and your new version of RcppCGAL and it builds (and works) without problems. You never know what's going to happen with CRAN's own package checks but I'm willing to take the risk and merge your PR.

@ericdunipace
Copy link
Contributor Author

I think they just changed the flag for the 6.0 version of CGAL, as far as I can tell. Without it, your package didn't build on my machine with the new headers (unless I link GMP); with the flag, everything seems ok. Let me know if you run into problems though! Happy to try and trouble shoot as much as possible.

@m-jahn m-jahn merged commit d261797 into m-jahn:master Nov 27, 2024
5 checks passed
@ericdunipace
Copy link
Contributor Author

FYI, I'm submitting the new version of RcppCGAL to CRAN, which will break the version of WeightedTreemaps there.

@m-jahn
Copy link
Owner

m-jahn commented Dec 10, 2024

Thanks for letting me know, and go ahead!
I will submit my updated package soon after to CRAN, so this should work out fine.

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.

2 participants