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 to tinyxml2 10.0.0 #288

Merged

Conversation

nilshoffmann
Copy link
Contributor

Installation of ggiraph fails on Alpine Linux 3.19 (within rhub/r-minimal:4.3) due to fseeko64 and ftello64 being picked (https://github.com/davidgohel/ggiraph/blob/master/src/tinyxml2.cpp#L109) during compilation.

Only fseeko and ftello are available (or legal) in Alpine / MUSL. This seems to have been fixed in tinyxml2.[h,cpp] version 10.0.0, or potentially earlier, but I opted to pick the latest release.

This PR replaces the old tinyxml2 files with the new ones. Compilation and usage of the package are then possible on Alpine Linux based systems.

The attached Dockerfile.alpine.txt uses one of the ggiraph demos to show that the patched version is loading and working in principle (on Alpine Linux).

You can build it with:

docker build --file Dockerfile.alpine.txt -t ggiraph-alpine --load

and run it with

docker run -p 3838:3838 -it ggiraph-alpine

You can then access the demo in your browser at localhost:3838.

@davidgohel
Copy link
Owner

Thanks a lot, much appreciated. I will take time to test next week.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.04%. Comparing base (0120933) to head (e458b25).
Report is 1 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #288   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files         106      106           
  Lines        2704     2704           
=======================================
  Hits         2543     2543           
  Misses        161      161           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidgohel
Copy link
Owner

Thank you, all seems ok to me. merging

@davidgohel davidgohel merged commit 3374ba0 into davidgohel:master Apr 11, 2024
9 checks passed
@davidgohel
Copy link
Owner

Hello @nilshoffmann

With latest version of R (>= 4.4), this new version of tinyxml2 trigger notes that will make CRAN deployment impossible:

* checking compiled code ... NOTE
File ‘ggiraph/libs/ggiraph.so’:
  Found ‘___stdoutp’, possibly from ‘stdout’ (C)
    Object: ‘tinyxml2.o’
  Found ‘_puts’, possibly from ‘printf’ (C), ‘puts’ (C)
    Object: ‘tinyxml2.o’

Compiled code should not call entry points which might terminate R nor
write to stdout/stderr instead of to the console, nor use Fortran I/O
nor system RNGs nor [v]sprintf.

I am sorry, for now, I'll revert that change.

KR,
David

davidgohel added a commit that referenced this pull request May 15, 2024
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.

3 participants