SergiusTheBest / FindWDK

CMake module for building drivers with Windows Development Kit (WDK)
BSD 3-Clause "New" or "Revised" License
251 stars 53 forks source link

Added link_directories() to wdk_add_driver() function #2

Closed sgeto closed 6 years ago

sgeto commented 6 years ago

Hey,

Please review this PR.

The main purpose of it is to allow users to use target_link_libraries() the same way they would when linking any other library/executable in cmake.

Meaning I added ${WDK_ROOT}/Lib/${WDK_VERSION}/km/${WDK_PLATFORM} to wdk_add_driver()'s library search path so that users only have to do for example:

target_link_libraries(mydriver ndis setupapi)

instead of

target_link_libraries(mydriver
"${WDK_ROOT}/Lib/${WDK_VERSION}/km/${WDK_PLATFORM}/ndis.lib"
"${WDK_ROOT}/Lib/${WDK_VERSION}/km/${WDK_PLATFORM}/setupapi.lib")

in theirCMakeLists.txt, which is something novice user would never figure out.

BTW if you think that adding ${WDK_ROOT}/Lib/${WDK_VERSION}/km/${WDK_PLATFORM} to link_directories() the way I did is going to pollute cmake's default library search path, then it isn't. I don't have an exact explanation as to why, but it seems as if everything in FindWdk.cmake is processed in an separate sub-shell and discarded after success/failure.

If you like it then we could add a test case for this as well.

SergiusTheBest commented 6 years ago

Hi!

Sorry for the late response. Somehow I missed the notification about this PR. Let me take a look at it. And thank you for your help!

sgeto commented 6 years ago

No problem 👍

SergiusTheBest commented 6 years ago

Unfortunately link_directories() affects another targets (as it should do according to the docs). I added a simple exe target and its link libraries path was polluted with WDK. It's not a big deal but confilcts are possible. I wish there were target_link_directories() in cmake.

I resolved linking to WDK libraries in another way. As modern cmake patterns suggest I created imported targets for WDK libraries and linking to them looks this way:

target_link_libraries(MinifilterCppDriver WDK::FLTMGR)

I think this is the best solution. Please, test it. Also I've added a MinifilterCppDriver to the samples and updated the README.