build cMocka lib manually to VCPKG#875
build cMocka lib manually to VCPKG#875vhvb1989 merged 5 commits intoAzure:masterfrom vhvb1989:path-cmocka-with-relative-path
Conversation
vhvb1989
requested review from
JeffreyRichter,
RickWinter and
joshfree
as code owners
cmake-modules/AddTestCMocka.cmake
Outdated
| set(CMOCKA_LIB "debug;${CMOCKA_PREXIF}/debug${CMOCKA_STATIC_LIB};optimized;${CMOCKA_PREXIF}${CMOCKA_STATIC_LIB}") | ||
| else() | ||
| set(CMOCKA_LIB cmocka) | ||
| set(CMOCKA_LIB cmocka) |
| set(VCPKG_PATH $ENV{VCPKG_ROOT}) | ||
| endif() | ||
| if(DEFINED ENV{VCPKG_INSTALLATION_ROOT}) | ||
| set(VCPKG_PATH "$ENV{VCPKG_INSTALLATION_ROOT}/") |
There was a problem hiding this comment.
Is the / at the end of the string intentional? Why don't we have the same thing above, in set(VCPKG_PATH $ENV{VCPKG_ROOT})
Also, is that why we surround it in quotes?
There was a problem hiding this comment.
it is intentional.
In the above lines, (usually Linux and Windows) the VCPK_ROOT includes the / at the end of the path
For VCPK_INSTALLATION_ROOT (Mac) the / is missing, that's why we add it.
the quotes are just required in the second one to make the concatenation. Above lines we don't need to
| set(VCPKG_PATH "$ENV{VCPKG_INSTALLATION_ROOT}/") | ||
| endif() | ||
|
|
||
| # Patch for new VCPKG cmocka returning relative path |
There was a problem hiding this comment.
Will we revert this once VCPKG issue is fixed (or maybe after the version pinning changes that @danieljurek was considering)?
There was a problem hiding this comment.
yeah, this is a temporal fix.
It is setting the path to cmocka libraries manually and hardcoded. This fix would work always while the name of the static library for cmocka doesn't change.
I am not sure if there is a bug for VCPKG, it might be that it used to be one and they fixed it and we were just making it work by luck wrongly before.
but, at any way, we need follow up on this
There was a problem hiding this comment.
thank you @danewalton .
That's good news.
I though cmocka could've done the change to work the same as when not using VCPKG.
when not working with VCPKG, cmake gets the path to the libs in a relative way, just like it is now now. That's why I thought that's how they wanted now.
So, as soon as VCPKG fix is merged, we can revert the changes here and it should work the same (and better without the hardcoded hot fix)
| if(MSVC) | ||
| set(CMOCKA_STATIC_LIB "/lib/cmocka-static.lib") | ||
| else() | ||
| set(CMOCKA_STATIC_LIB "/lib/libcmocka-static.a") |
There was a problem hiding this comment.
Question: Is the name of the .a file accurate? Is it supposed to be libcmocka-static?
There was a problem hiding this comment.
it is .a on Posix (lin/mac) and .lib on windows.
the expectation is that cmoka will use whatever is loaded on ${CMOCKA_LIBRARIES}. That variable is provided by VCPKG when cmocka is installed. Cmocka provides the scripts to VCPKG to generate this variable.
In the past, this variable was generated like:
degug;/path/to/VCPKG/{vcpkg packages installed path}/libraries;optimized;/path/to/VCPKG/{vcpkg packages installed path}/libraries
but cmocka made a change and now it looks like:
debug;/libraries;optimized;/libraries
So the prefix path to the libraries was removed from both, debug and optimized libs.
I am just manually building the value we used to have before
| else() | ||
| set(CMOCKA_STATIC_LIB "/lib/libcmocka-static.a") | ||
| endif() | ||
| set(CMOCKA_PREFIX "${VCPKG_PATH}installed/${VCPKG_TARGET_TRIPLET}") |
There was a problem hiding this comment.
Question: Does this need a directory separator?
| set(CMOCKA_PREFIX "${VCPKG_PATH}installed/${VCPKG_TARGET_TRIPLET}") | |
| set(CMOCKA_PREFIX "${VCPKG_PATH}/installed/${VCPKG_TARGET_TRIPLET}") |
There was a problem hiding this comment.
no, VCPKG_PATH is expected to have the / at the end
| set(CMOCKA_STATIC_LIB "/lib/libcmocka-static.a") | ||
| endif() | ||
| set(CMOCKA_PREFIX "${VCPKG_PATH}installed/${VCPKG_TARGET_TRIPLET}") | ||
| set(CMOCKA_LIB "debug;${CMOCKA_PREFIX}/debug${CMOCKA_STATIC_LIB};optimized;${CMOCKA_PREFIX}${CMOCKA_STATIC_LIB}") |
There was a problem hiding this comment.
Question: How did you know this is what we need to set CMOCKA_LIB to? Will we always have debug?
I am trying to understand what this means.
There was a problem hiding this comment.
no,
this is how a package tells VCPKG where to find libraries for debug and for optimized.
The string looks like degug;path;optimized;path
Then, depending on what cmake is building, it will pick debug or optimized.
Then one on optimized does not have the debug/
This reverts commit 90c9007.
4 participants
Fixes the current regression on Mac.