Skip to content

[v-hacd] Fix cmake patch for include files#13549

Merged
BillyONeal merged 2 commits intomicrosoft:masterfrom
FabienPean:master
Oct 9, 2020
Merged

[v-hacd] Fix cmake patch for include files#13549
BillyONeal merged 2 commits intomicrosoft:masterfrom
FabienPean:master

Conversation

@FabienPean
Copy link
Contributor

This PR is an attempt at fixing the v-hacd port. It fixes the target include directory so that the public include file VHACD.h can be found in the include path. It also stops installing the private header files in order to reduce clutter.

I think the patch+portfile from the port (and also the CMakeLists from the project repository) could benefit from some rework (cleanup, simplification, fix of shared library), unfortunately I am unable to do it at the moment. The changes are the minimum I had to do to make it work on my machine.

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    Tested with x64-windows and VS2019. What is the CI baseline ?

Let me know if anything is missing or feel free to take over from here.

@FabienPean FabienPean marked this pull request as ready for review September 15, 2020 21:09
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Please also modify the CONTROL file to introduce a Port-Version: 1 below the current Version: field.

@PhoebeHui PhoebeHui added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Sep 16, 2020
@FabienPean
Copy link
Contributor Author

Thanks for the review, I amended the commit with the requested changes.

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR!

@PhoebeHui PhoebeHui added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Sep 17, 2020
@PhoebeHui
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal BillyONeal merged commit d192905 into microsoft:master Oct 9, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants