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

Fix nativefiledialog port using build time include dir #18923

Merged
merged 7 commits into from Jul 16, 2021

Conversation

jfhs
Copy link
Contributor

@jfhs jfhs commented Jul 12, 2021

Describe the pull request

  • What does your PR fix?

Fixes installed lib include path pointing to build-time location. Problem is in this bit:

target_include_directories(
    nfd
    PUBLIC
        $<BUILD_INTERFACE:
            ${CMAKE_CURRENT_LIST_DIR}/src
            ${CMAKE_CURRENT_LIST_DIR}/src/include
        >
        $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

For install target, BUILD_INTERFACE evaluates to empty string, which is expanded to current build folder and so unofficial-nativefiledialog-config.cmake has lines like these that include absolute build folder path:

set_target_properties(unofficial::nativefiledialog::nfd PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "C:/src/vcpkg/buildtrees/nativefiledialog/src/44c798f8b9-0d908eb5b1.clean/;${_IMPORT_PREFIX}/include"
)

Separating into two different target_include_directories fixes it.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    All (except UWP), No

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

@ghost
Copy link

ghost commented Jul 12, 2021

CLA assistant check
All CLA requirements met.

@jfhs jfhs force-pushed the jfhs/fix-nativefiledialog branch from 45a1f5f to 4394d9b Compare July 12, 2021 22:35
@jfhs jfhs force-pushed the jfhs/fix-nativefiledialog branch from 4394d9b to 29f9654 Compare July 12, 2021 22:47
@jfhs jfhs marked this pull request as ready for review July 12, 2021 22:59
ports/nativefiledialog/vcpkg.json Outdated Show resolved Hide resolved
ports/nativefiledialog/vcpkg.json Outdated Show resolved Hide resolved
versions/baseline.json Outdated Show resolved Hide resolved
versions/n-/nativefiledialog.json Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Jul 13, 2021
jfhs and others added 4 commits July 13, 2021 10:54
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
versions/n-/nativefiledialog.json Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines labels Jul 14, 2021
@vicroms vicroms merged commit 5236efa into microsoft:master Jul 16, 2021
@vicroms
Copy link
Member

vicroms commented Jul 16, 2021

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants