-
Notifications
You must be signed in to change notification settings - Fork 684
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
CMake support? #164
Comments
As a note, I am currently suggesting an additive change, not one that would necessarily need to replace what you currently have, though a project can always build PcapPlusPlus with cmake and include it in their projects manually similarly to how you currently have users build it manually and use it by setting linker/include paths. |
Thanks for your suggestion! You're actually not the first one who brought this up. I had some discussions about it with @solvingj as well (Jerry - please feel free to chime in). I'm not against CMake, but I want to make sure we keep an option to build PcapPlusPlus without CMake as a dependency. That's why the additive change you're suggesting sounds interesting. If we go with this suggestion let's make sure that:
As a next step my suggestion would be to create a fork from the dev branch, implement CMake and start testing it on all platforms. I'll provide all the help you need. When we both feel comfortable with it we'll can create a PR and I'll merge it into dev and then master. Please let me know what you think |
Indeed, cmake makes libraries more accessible for integration and inclusion in various ecosystems, including enterprise use cases. Sadly, there are a number of cautionary tales of “community supported cmake files.” The final comment by GamePad64 actually summarizes a similar origin story to this conversation. I think we can learn from this example and hopefully avoid wasting anyone’s time. In short, one of CMake’s primary value propositions is to reduce the burden and complexity of maintaining portable cross platform build instructions: both for maintainers and consumers. It only delivers this value when it replaces the existing build instructions as the single supported system by the project maintainer. Any other approach results in the opposite outcome: doubling the net maintenance burden, and fracturing the support conversations. It’s strictly worse IMO. I don’t intend this to be taken negatively, only honestly. |
Thanks for your comments @solvingj , I read almost the entire thread you shared and I can understand both sides. CMake has its pros and cons. It's probably easier to use in Linux and MacOS (actually I'm not sure about Linux because of its many distros) but Windows is not always easy. It's very clear from the thread that CMake is not a consensus. Some people love it, some people hate it; Some tools adopt it, some don't. I know CMake's popularity has increased in the last few years, but I don't feel it has become a de-facto standard yet (like Maven/Gradle for Java or Cargo for Rust). That's why I suggested using both build systems. Then I read your comment and GamePad64's summary about how non-efficient it'd be to support both build systems. I'm not sure which side to pick... on one hand: if this would make the life of (at least some of) PcapPlusPlus's users easier then why not providing it to them and keep the current build system for the users who don't like CMake. On the other hand, as you said, maintaining 2 build systems would be a burden. I'm not sure what would be the right path to choose. What do you think? |
Glad that I've at least piqued your interest. So here's my perspective, but first some answers to your questions/points :-).
So, my perspective... I agree that there is no consensus around cmake, but it appears to be growing. Enough that major players such as qt-creator, clion, and visual studio have first class support for it. It isn't a perfect tool for the job, but it seems to be one of the better ones available, and I don't personally see it being overtaken by something even newer anytime soon. Additionally, while there may be no consensus, cmake is fairly ubiquitous at this point. I have yet to see a distro without a cmake package in recent years. So, this would be an optional build dependency on a tool which has a near 100% chance of already being present. The only downside that I really see is that my solution suggests maintaining two sets of build scripts, (assuming that you guys continue to personally favor the custom solution and don't jump on the bandwagon yourselves in the future ;-) ). To which I ask: How often do you update your build scripts anyway? If it's very infrequent, then having to maintain two shouldn't be too bad IMO, if it's frequent, well, maybe my suggestion should wait until development settles down to a more stable situation and revisit the idea later. Anyway, I won't be offended if the answer is "no thanks", honestly, my main motivation is that it would make my life easier in the future. I like the idea of being able to pull the latest version of my dependent libs and not have to worry about it conflicting with changes I've made to a local copy. Better is to have upstream support my needs and just be able to pull down the latest without thinking! Let me know what you guys think. |
@eteran Thanks for your comments! I really appreciate your efforts and your offer to help. I'm glad to hear you already have something in place, and that you plan to complete the work for the remaining platforms and dependencies. I really hope that adding the tests and examples will be as straight-forward as you estimate. The discussion around CMake vs GNUMake is long going and I'm not sure we'll be able to decide one way or the other. All options have pros and cons (keep only GNUMake, move completely to CMake or maintain both). While it's important to keep this discussion alive, I think it'll also be beneficial to start some work and see where it leads us. Regarding your question about build system changes: as you can see we have quite a lot for Linux: But quite a few for the others: So my suggestion would be: let's create a new fork from the Please let me know what you think. |
17 commits in 5 years, I think I can handle that level of updates for cmake, especially since some of the types of things in those commits are things that cmake "just does" (install paths for example). So yea, I'll fork from dev, and hopefully it won't be long before I have something worth showing soon! |
Sounds great, thanks again for your efforts and help! |
I have a question :-) How much should I try to avoid changing things in the 3rdParty directory, specifically LightPcapNg? Their existing cmake file unfortunately seems unaware that cmake handles choosing static vs shared linking already and has a bunch of duplicate work as a result. Also, I would assume that if I leave it as is, then PcapPlusPlus wants the static build of this library? My preference is to "correct" their cmake file, but, I don't want to make things more complex than necessary. |
I suggest fixing their cmake file, and at a later time you can choose whether to suggest a PR for LightPcapNg as well. When I started using LightPcapNg I also made some code changes and suggested them to the author and he accepted them |
I have an update! I have it building for linux, with examples and tests. Currently DPDF and PFRing are untested, but the groundwork is there. I have also implemented a "make test" target, and Packet++-Test passes perfectly :-) I was wondering how/if Pcap++-Test should be run to get a pass/fail. (If it's a different kind of test, that's also fine of course). If you're curious what the build system looks like, feel free to take a look at my dev fork here: https://github.com/eteran/PcapPlusPlus/tree/dev |
Thanks @eteran ! Do you have also the Windows and MacOS version working? I can help with testing PF_RING and DPDK. Regarding Pcap++Test - you can run it in 3 modes:
|
Linux only for now, but windows, osx, ming-gw are coming next 🙂. |
Awesome thanks! |
Update!, PF_RING support is looking good on linux, you set PF_HOME and it find everything, builds and links. DPDK is slightly more complex as I can't build it on my non-numa box it seems. So I'm looking into how to work around that (suggestions are of course welcome :-)). At this point, I would say feel free to clone my repo and play with it if you want to test things on linux. I Highly recommend installing the cmake-gui package as it will install an ncurses gui for cmake letting you see the options which can be toggled. |
Thanks for the update. Regarding DPDK - you don't have to have a numa box to build it. Any Linux box should do, even a VM. Building it it pretty easy:
If you have a standard Linux 64-bit machine you can set |
Hmm, I did that and my laptop fails to compile with missing numa.h header. Maybe it's a gentoo thing, I'll try in a Ubuntu docker later :-) |
@eteran You must install ‘’’libnuma-dev’’’ package and some other (see installment guide on DPDK official site) |
Just wanted to give an update since it's been a bit :-). Been busy with work. But I have everything working on linux including DPDK builds! The only improvement I have left on the linux build is to auto-detect which compiler flags to use instead of assuming Once I have that in order, I'll probably try to tackle the more interesting target... Windows! |
Awesome @eteran thank you so much for your help! I was also busy with various things but I'll try to make some more time for it in the near future. Please let me know once you finish the Linux changes and I'll chime to help you test and add CI jobs targeted to Cmake. I can also help with Windows |
@eteran I'd like to contribute but I cannot fork your repo (because it's a fork of my repo). How do you suggest I contribute? Maybe we can merge this code to a dedicated branch in |
Wow, it seems CMake support is on the way. Awesome! |
@seladb, a dedicated branch definitely seems like a good idea. I think can then work on that branch in my fork to merge in as needed going forward. Let me know if there is anything you need me to do to help with that part. |
I'm happy too... but I'm not a git expert. So it may take me a bit to figure it out |
I think you can simply open a new PR from your fork |
@eteran I'm adding The information I'm missing is how to provide btw, I've added UPDATE: I found out how to do this and I've added |
@eteran I've started working on Windows (both VS and MinGW) but my progress is very slow because of my poor familiarity with CMake. Do you think you'll be able to help? |
Absolutely. I have a windows box with visual studio setup. |
Awesome thank you! |
do you already know when you'll have time to work on that? |
Soon, I'm in the middle of a move, so it's been a little hectic. But it should settle down shortly :-). |
is there any progress on the windows side of the cmake building availability? |
I apologize for the delay. I was mid-move in June, and this just kinda fell through the crack. I'll see what I can do to get back into finishing this, honestly, it's probably pretty close as the primary challenge is the system-specific library requirements (like finding and linking to WinPcap). |
@eteran I made some progress on top of your work. I think the main challenge remaining is around Windows support (both MinGW and VS). Also there have been a lot of changes in |
How should i link the pthreads and winpcap-dev-kit using the cmake project? |
@agentsofshield which cmake project are you referring to? is it the work-in-progress that exists on the |
@seladb yup to the cmake branch |
Windows support is not yet really working, it is still work in progress, please feel free to add that part :) |
hi @eteran , any progress on that? I can't wait to use cmake for this project. |
@DinoStray we haven't made much progress on this for a while. And from my experience it's not very easy to make cmake work on all platforms with the different dependencies each platform has. To be very honest, I'm not even sure it's worth the effort because the cmakefile complexity will be as much if not more than the current configuration scripts. |
Hey guys, sorry for the radio silence. Honestly, the impetus for this was originally a work requirement to make any changes we do to 3rd party code accepted upstream.. which has since been relaxed. So I just haven't had the need to add support for the other platforms lately. However, I can say that I would love to see it make it all the way in eventually, as PCPP is a great project that could be used a lot more widely if it had a more standardized build system that could integrate with existing projects... like cmake :-). I do disagree with @seladb that the cmake files add much complexity, they are even in the cmake branch, significantly shorter and more declarative than the custom scripts already. The only real complexity comes from the "FindXXX" modules, which aren't too bad at all IMHO. So, to summarize ... :-) I'd love to get this across the finish line, I think it'll be worth the effort and help more users take advantage of the lib... the main hold up has been a change in work-related priorities. We used to consider getting all patches upstream to be VERY important for maintenance, but not as much since, in practice, it hasn't been a real detriment. |
@eteran thanks for your comment! As discussed earlier on this thread, I'd love to see cmake added as another build option for PcapPlusPlus. We were already about half way through with the implementation but unfortunately no one had the time to complete the work. If someone on this thread is willing to take it up that'd be great and I'll help in any way that I can. |
I think we should consider separate linux and windows build. |
We can definitely do it, but there is a demand for both platforms 😃 |
@eteran @DinoStray @agentsofshield Can I have your review on the Cmake support PR ? |
Merged into dev with #944 |
Hello, and thanks for your work!
I am using PcapPlusPlus in a project, and we use cmake for our build system. I have written some simple, modern cmake files which allow your work to be easily included in existing source trees that use cmake; for example as a github sub-module.
Would you have any interest in a PR to add support for it upstream? Currently, I've only tested it on Linux, but I don't think it would be difficult to deal with Windows/macOS as well.
The text was updated successfully, but these errors were encountered: