-
Notifications
You must be signed in to change notification settings - Fork 639
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
Build Shared Libs (.dll) on Windows with MSVC #1101
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1101 +/- ##
==========================================
- Coverage 81.40% 81.00% -0.41%
==========================================
Files 148 148
Lines 19220 18702 -518
Branches 7233 6285 -948
==========================================
- Hits 15647 15150 -497
+ Misses 3157 3137 -20
+ Partials 416 415 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 77 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Cool thank your for the MR :) I'm not in the Windows env and really don't know but would it not be easier to do something like this directly in the Cmake if WIN32 and Compile_Shared and Export Leave the PACPP_EXPORT empty for everybody else And use PACPP_EXPORT in the source code, this will avoid to introduce a CommonExport.h |
@clementperon Hello, here is the reference https://stackoverflow.com/questions/27429732/cmake-adds-dlibname-exports-compile-definition#:~:text=cmake%20add%20%3Clibname%3E_EXPORTS%20macros%20only%20for%20shared%20libraries.%20It%27s%20useful%20when%20exporting%20API%27s%20in%20Windows%20DLL.. CMake will automatic add Here also be a good example: https://github.com/SFML/SFML/blob/master/include/SFML/Config.hpp#:~:text=export%20and%20import-,%23,(dllimport),-// |
Yes I agree you're code is correct, I'm just looking for something simpler. Just found this while looking :) |
@clementperon Yes, you are right. |
@traversebitree also would be nice to add a windows job with DLL built in CI, thanks. Don't hesitate if you need help ! |
@clementperon OK, I'll have a try. |
|
I guess |
temporary stop memplumber
@traversebitree are these the only tests which fail in |
Recover test
Test disable memleakcheck
@seladb I test to manually set Packet++Test.exe:
Pcap++Test.exe (run stuck, and I stopped the action process):
|
@traversebitree I think I tried to build shared libs for Windows many years ago and stumbled into a somewhat similar issue. If I remember correctly, there was an issue when a DLL and an external app share memory - e.g the DLL gives direct access to its memory to the external app, or the app gives direct access to its memory to the DLL. This happens quite a lot in PcapPlusPlus. For example here the DLL shares a pointer to its internal memory with the external app (in this case the test app): I'm not at all sure these are the issues you see, I just remember them vaguely from many years ago. If this is indeed the issue, it'd be very hard to solve because PcapPlusPlus uses this memory sharing in many places in its API. |
Test add `MTd` CXX Flag
@egecetin Yes, I also noticed this problem, so I added this |
@traversebitree what should we do with this PR? |
@seladb The fact is that building .dll is not compatible with MemPlumber. When the memory check is skipped, the building .dll is valid and passes the test. So, I don't know whether to go ahead and modify the MemPlumber or terminate this PR. I've been working on my paper recently, so I haven't been able to continue working on this PR lately, sorry about that. |
@traversebitree don't we have this issue as well? #1101 (comment) that is unrelated to MemPlumber? |
I'm sorry, but after trying too many times, it still doesn't pass the memory leak test. |
Dynamic library on Windows cannot be generated before. So I made some changes to do that.
Major changes:
CMakeLists.txt
, add propertiesWINDOWS_EXPORT_ALL_SYMBOLS
,POSITION_INDEPENDENT_CODE
for building.dll
on Windows, and add macroPCPP_tgt_BUILD_DLL
for determining if build shared libs (tgt
is target's name). Following is an example.tgtExport.h
(tgt
is target's name). For example:CommonExport.h
. For building.dll
shared libs, we need to declare__declspec(dllexport)
and__declspec(dllimport)
at the beginning of the declarations that are being exported. And define the macroPCAPPP_tgt_API
to it (When using linux or build static libs, will ignore the macro). And settgtExport.h
in public headers inCMakeLists.txt
.In nearly every header file, add
#include "tgtExport.h"
at begin, and addPCAPPP_tgt_API
ahead of each static member of class, for example:static const IPv4Address Zero;
change toPCAPPP_COMMON_API static const IPv4Address Zero;
. refer to https://www.kitware.com//create-dlls-on-windows-without-declspec-using-new-cmake-export-all-feature/.Add "copy
.dll
to build directory and install.dll
to install directory" for every example target. Only ifBUILD_SHARED_LIBS
andWIN32
, will copy.dll
to the targets folder. Following the example:Minor changes:
.gitignore
, addcompile_commands.json
and.cache
for programmers who useCMake
andClangd
.Then we can build shared libs (.dll) on Windows using MSVC with option
BUILD_SHARED_LIBS
isON
.