-
Notifications
You must be signed in to change notification settings - Fork 13
Switch to vcpkg to fetch dependencies #96
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
Conversation
989060f
to
a145853
Compare
Hey. I think that CI jobs fail because by default, GitHub doesn't check out submodules. We'd need |
6c0c19b
to
dd851d6
Compare
Change-Id: Ic1a6ef5cf5af5b0567817392ce5fc61358b690ca
We declare the default repository with appropriate sha separately in vcpkg-configuration.json. Having "builtin-baseline" in such case generates vcpkg warning.
We don't want to check the code there, that's not our code.
Was vcpkg/vcpkg due to omission. Also, it was made shallow by default. Normally people will just want to build so there's no need to fetch it with the whole history. If someone wants to work with the submodule, it can always be unshallowed.
Otherwise, we won't get vcpkg.
Like we do in one of our internal projects. This way it will be less confusing for the user and they'll get instant feedback on what to do if they checked out without submodules.
Now vcpkg does this.
For now we only test with external dependencies on macOS. mio and whereami are not available in brew so we'll still fetch them with CMake. Doctest needed to be added to brew deps.
It didn't use the FindWhereami.cmake file because the capitalization was wrong. In find_package() we used "whereami", starting with lowercase letter.
I cannot figure out why they started popping up after switching to vcpkg. Seems that without vcpkg tests were not actually going through clang-tidy for some reason. Let's fix those then.
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.
My review pertains only to Paul's commit as it was me who built upon it so I'm hereby not reviewing my own commits.
vcpkg.json
Outdated
"description": "KD Utilities Library - Core utilities and helper functions", | ||
"homepage": "https://github.com/KDAB/kdutils", | ||
"license": "MIT", | ||
"supports": "!(uwp | arm)", |
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.
!arm is a bit sus
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.
Yup, overlooked it. Now I made it more truthful.
It's just informative but let's at leas be truthful here. It doesn't mean it won't work elsewhere. Just that these are the platforms we currently test for and care about. May change in the future.
vcpkg should hopefully be more convenient than FetchContent.
In turn KDGpu and other internal projects will be able to build KDUtils with vcpkg