r-lib / ragg

Graphic Devices Based on AGG
https://ragg.r-lib.org
Other
172 stars 24 forks source link

Prefer using PKG_CPPFLAGS for ragg headers #125

Closed kevinushey closed 1 year ago

kevinushey commented 1 year ago

From https://cran.r-project.org/doc/manuals/R-exts.html#Using-Makevars:

N.B.: Include paths are preprocessor options, not compiler options, and must be set in PKG_CPPFLAGS as otherwise platform-specific paths (e.g. ‘-I/usr/local/include’) will take precedence. PKG_CPPFLAGS should contain ‘-I’, ‘-D’, ‘-U’ and (where supported) ‘-include’ and ‘-pthread’ options: everything else should be a compiler flag. The order of flags matters, and using ‘-I’ in PKG_CFLAGS or PKG_CXXFLAGS has led to hard-to-debug platform-specific errors.

And also:

Use #include "my.h" not #include for headers in your package. The second form is intended for system headers and the search order for such headers is platform-dependent (and may not include the current directory). For extra safety, name headers in a way that cannot be confused with a system header so not, for example, types.h.

Because Makevars.in sets the include path via PKG_CXXFLAGS here:

https://github.com/r-lib/ragg/blob/f6acc4c89540a71743074455d5c93e7f48ff075e/src/Makevars.in#L1

we run the risk of picking up an alternate installation of agg, rather than the bundled version. It should suffice to just set the include paths in PKG_CPPFLAGS rather than updating how agg is included, though.