-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[dlib] fix problematic public target flags #49056
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
base: master
Are you sure you want to change the base?
[dlib] fix problematic public target flags #49056
Conversation
|
@microsoft-github-policy-service agree company="DeepForm Limited" |
1 similar comment
|
@microsoft-github-policy-service agree company="DeepForm Limited" |
|
This is probably good but as I'm not 110% sure I'm going to give upstream some time to look. (I'm on vacation at the moment so I'm being more cautious than usual :) ) |
|
That's valid; no need to rush it 😁 It'll be good to get more eyes on this after the new year. One potential caveat on waiting for upstream is that even if davisking/dlib#3126 is merged, it won't become available to vcpkg until a new patch/version of dlib is published, since the port pins by version tag (unless the port is adjusted to reference a specific commit instead). As a result we may still be waiting on upstream for quite a while. Upon further thinking, since #49035 is a build error which particularly impacts vcpkg (due to the ease of switching compilers, such as by having a different target triplet from the host), I think it would be useful to have the patch present in vcpkg until dlib publishes a release with the fix anyways. Reason being: In absence of a fix, it's actually impossible to build to any non-MSVC triplet right now if the host is using MSVC (based on my understanding). This also provides the benefit of allowing consumers to pin to v20.0 without issue, in case future versions of dlib causes other bugs / incompatibilities. |
BillyONeal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential caveat on waiting for upstream is that even if davisking/dlib#3126 is merged, it won't become available to vcpkg until a new patch/version of dlib is published
If upstream is happy with the edits as this seems to be mostly build system changes I think we can take the patch. Just trying to avoid "putting words in their mouth" so to speak; if they merge it themselves that means they're happy and as a result I'm happy.
Mitigates #49035 (pending a fix to davisking/dlib#3125 or merge of davisking/dlib#3126)
./vcpkg x-add-version --alland committing the result.