feat: Let cpp_register() find the types header in src/include/ - #9
Open
krlmlr wants to merge 1 commit into
Open
Conversation
`cpp_register()` looked for `{package}_types.h(pp)` in `src/` and in
`inst/include/` only, so a package whose generated `src/cpp11.cpp` must
include that header had to install it -- turning a private header into
part of the package's public interface.
Add `src/include/` to the search, and include the header by its path
relative to `src/` so no extra include flag is needed.
See duckdb/duckdb-r#2501
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Patch
f-types-src-includeof the stack. New in this round — it is the patch duckdb/duckdb-r#2501 needs.cpp_register()looked for{package}_types.h(pp)insrc/and ininst/include/only. A package that keeps its own headers insrc/include/— a common enough layout, andduckdb-r's — therefore had to put that one header ininst/include/to be found, which installs it and turns a private header into part of the package's public C++ interface.duckdb-rcalls its copy a vendoring accident for exactly that reason.src/include/joins the search, between the two existing locations so the relative precedence ofsrc/overinst/include/is unchanged.Emitting the include
A header found in
src/include/is included as#include "include/foo_types.hpp", not by basename. The generated file issrc/cpp11.cpp, and a quoted include resolves relative to the including file's own directory first, so this works with no-Iflag — a package need not have arranged forsrc/include/to be on the include path. Headers found in the other two locations are still included by basename, so their output is byte-identical to before.Verification
Two tests added beside the four existing
pkg_typescases, one per extension. Fulltest-register.Rpasses, as does the rest of the R suite.Worth sending upstream on its own: nothing about it is specific to this fork, and if
r-lib/cpp11takes it the stack carries one patch less.🤖 Generated with Claude Code
https://claude.ai/code/session_01K8MneV8KqHYUuC8fWV3X5Q
Generated by Claude Code