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

buildsystem: include directories vs target sources #43

Closed
BurningEnlightenment opened this issue May 24, 2022 · 3 comments
Closed

buildsystem: include directories vs target sources #43

BurningEnlightenment opened this issue May 24, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@BurningEnlightenment
Copy link
Collaborator

The vcpkg port moves all headers into a status-code subdirectory:
file(RENAME "${CURRENT_PACKAGES_DIR}/include2" "${CURRENT_PACKAGES_DIR}/include/status-code")
https://github.com/microsoft/vcpkg/blob/eddbb406cff0ff7828592d5a92e64abb59808534/ports/status-code/portfile.cmake#L25-L26

This, however, is incompatible with the install interface target sources which expect the headers to directly reside in the include directory:

status-code/CMakeLists.txt

Lines 109 to 111 in 42d3afa

target_sources(status-code INTERFACE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${source}>"
"$<INSTALL_INTERFACE:${source}>"

Additionally I find install interface target sources to be annoying, because e.g. the cmake VS solution generator adds them to all (transitively) depending targets. What was the rationale for referencing all headers from the installed target? Can we do without?

@ned14 ned14 added the bug Something isn't working label Jun 23, 2022
@ned14
Copy link
Owner

ned14 commented Jun 23, 2022

Am I being dumb here or isn't the right fix just to fix how cmake does the install so it's right, and then the vcpkg port doesn't need to rename anything?

@BurningEnlightenment
Copy link
Collaborator Author

Yes, probably. Is it intended to look like #include <status-code/status_code.hpp> or #include <status_code.hpp>?

Anyway, can we do without the target sources for installed targets? My issue looks like this:
grafik

I'd make a PR.

@ned14
Copy link
Owner

ned14 commented Jul 27, 2022

Note that we can't update Outcome to match this until after the pending Boost release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants