PcapPlusPlus icon indicating copy to clipboard operation
PcapPlusPlus copied to clipboard

CMake support?

Open eteran opened this issue 5 years ago • 47 comments

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.

eteran avatar Feb 27 '19 02:02 eteran

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.

eteran avatar Feb 27 '19 02:02 eteran

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:

  • It works for all supported platforms (Linux, MinGW, MinGW-w64, Visual Studio, MacOS)
  • It works side by side the current build system
  • It works with all dependencies:
    • WinPcap and Pthreads for Windows
    • libpcap for Linux and MacOS
    • DPDK (Linux only)
    • PF_RING (Linux only)
  • We provide CMake build for the entire project:
    • 3 libraries (Common++, Packet++, Pcap++)
    • 2 test projects (Packet++Test, Pcap++Test)
    • All examples
    • If possible, tutorial examples
  • We have CI tests for that (Travis CI and AppVeyor)
  • We have someone who can maintain this over time. Would you be willing to help here?

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

seladb avatar Feb 27 '19 05:02 seladb

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.”
Crypto++ is a recent case I got involved, but there are many more. https://github.com/noloader/cryptopp-cmake/issues/1

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.

solvingj avatar Feb 27 '19 07:02 solvingj

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?

seladb avatar Feb 27 '19 10:02 seladb

Glad that I've at least piqued your interest. So here's my perspective, but first some answers to your questions/points :-).

  1. I do plan for it to work on all supported platforms. Visual Studio is actually pretty easy as you can just open a CMakeList.txt file in it and it treats it like a project automatically. Does it have to work with all out of the gate? Or can it be incremental?
  2. I won't touch the current build system and it should have no conflicting files. It will work side by side.
  3. It should be easy to have it work with dependencies. There are sensible package finding scripts for most things I've needed in the past, and it isn't hard to write new ones where they are lacking.
  4. Right now I have it building and link each internal lib separately already, adding the tests and tutorials should be trivial. This will enable a make test target for easy regression testing.
  5. I have experience using cmake with Travis CI and AppVeyor, shouldn't be a problem.
  6. Willing to help of course. It should mostly only really need maintenance when source files are added/removed which typically is a relatively infrequent event. in the grand scheme of things. And updating the scripts is typically very easy.

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 avatar Feb 27 '19 14:02 eteran

@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: https://github.com/seladb/PcapPlusPlus/commits/master/configure-linux.sh

But quite a few for the others: https://github.com/seladb/PcapPlusPlus/commits/master/configure-windows-mingw.bat https://github.com/seladb/PcapPlusPlus/commits/master/configure-windows-visual-studio.bat https://github.com/seladb/PcapPlusPlus/commits/master/configure-mac_os_x.sh

So my suggestion would be: let's create a new fork from the dev branch, add CMake support to it and complete all tests. Once we have a working version that we're all comfortable with it may be easier for us to make a decision.

Please let me know what you think.

seladb avatar Feb 27 '19 20:02 seladb

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!

eteran avatar Feb 27 '19 21:02 eteran

Sounds great, thanks again for your efforts and help! Please keep us posted.

seladb avatar Feb 27 '19 22:02 seladb

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.

eteran avatar Feb 28 '19 15:02 eteran

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

seladb avatar Feb 28 '19 16:02 seladb

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

eteran avatar Mar 05 '19 20:03 eteran

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:

  • If you have network connectivity then you should pass the interface name or interface IP address that can accept and send traffic, for example: Bin/Pcap++Test -i 10.0.0.1 or Bin/Pcap++Test -i eth0
  • If you don't have network connectivity you can run it in a "no network" mode: Bin/Pcap++Test -n
  • For testing with DPDK you should pass both an interface name/IP address and DPDK interface ID, for example: Bin/Pcap++Test -i 10.0.0.1 -k 0

seladb avatar Mar 05 '19 22:03 seladb

Linux only for now, but windows, osx, ming-gw are coming next 🙂.

eteran avatar Mar 05 '19 23:03 eteran

Awesome thanks! I can help with DPDK and PF_RING, please let me know when it's the right time

seladb avatar Mar 05 '19 23:03 seladb

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.

eteran avatar Mar 22 '19 15:03 eteran

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:

make config T=[platform_type_string] && make

If you have a standard Linux 64-bit machine you can set T to be x86_64-native-linuxapp-gcc

seladb avatar Mar 22 '19 16:03 seladb

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 avatar Mar 23 '19 00:03 eteran

@eteran You must install ‘’’libnuma-dev’’’ package and some other (see installment guide on DPDK official site)

echo-Mike avatar Mar 23 '19 05:03 echo-Mike

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 -msse3 or similar.

Once I have that in order, I'll probably try to tackle the more interesting target... Windows!

eteran avatar Apr 24 '19 19:04 eteran

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

seladb avatar Apr 24 '19 22:04 seladb

@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 seladb/PcapPlusPlus and then both of us will be able to add code?

seladb avatar May 14 '19 06:05 seladb

Wow, it seems CMake support is on the way. Awesome! It seems Windows will be the final "boss", more complex than under *nix. I mainly work under Windows, so if necessary, I can test for you. :-P

tan-wei avatar May 15 '19 03:05 tan-wei

@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.

eteran avatar May 16 '19 19:05 eteran

Thanks @eteran , I've created the branch cmake from dev, could you please merge your code to that branch?

seladb avatar May 16 '19 20:05 seladb

I'm happy too... but I'm not a git expert. So it may take me a bit to figure it out

eteran avatar May 16 '19 20:05 eteran

I think you can simply open a new PR from your fork eteran/PcapPlusPlus to the new cmake branch on seladb/PcapPlusPlus. Once I merge this PR then all of your code will be there and we can work together on this branch

seladb avatar May 16 '19 20:05 seladb

@eteran I'm adding cmake builds to TravisCI. Could you please tell me how what is the cmake command to run for building with DPDK and PF_RING?

The information I'm missing is how to provide DPDK_HOME and PF_RING_HOME to cmake. Is this done through the cmake command line? or maybe using env variables?

btw, I've added cmake builds for Ubuntu 18.04 and Fedora 29 (without DPDK/PF_RING) and both are passing. I've also add a MacOS build but it's failing. I'll try to look into it

UPDATE: I found out how to do this and I've added cmake DPDK and PF_RING builds to Travis-CI, and I'm happy to announce these builds are passing!! As a next step I'll look into MacOS builds

seladb avatar May 25 '19 09:05 seladb

More updates: I've fixed the MacOS issues and the build is now running successfully on Mac. I've also added cmake builds to Travis-CI.

@eteran as I'm a beginner on CMake, could you please review my commit and let me know if you have any comments? https://github.com/seladb/PcapPlusPlus/commit/d2665fb78e3cdaa7eeec7fab67cecf4578ed2a1e

seladb avatar May 28 '19 20:05 seladb

@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?

seladb avatar May 31 '19 23:05 seladb

Absolutely. I have a windows box with visual studio setup.

eteran avatar Jun 01 '19 05:06 eteran

Awesome thank you!

seladb avatar Jun 01 '19 06:06 seladb

do you already know when you'll have time to work on that?

seladb avatar Jun 07 '19 15:06 seladb

Soon, I'm in the middle of a move, so it's been a little hectic. But it should settle down shortly :-).

eteran avatar Jun 09 '19 12:06 eteran

is there any progress on the windows side of the cmake building availability?

agentsofshield avatar Oct 06 '19 08:10 agentsofshield

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 avatar Oct 07 '19 02:10 eteran

@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 dev/master branches so we need to sync up the cmake branch with dev

seladb avatar Oct 07 '19 05:10 seladb

How should i link the pthreads and winpcap-dev-kit using the cmake project? Should I just set the THREADS_PTHREADS_INCLUDE_DIR variable and the corresponding one for winpcap and it will run, or are there more missing features to be created for it to compile and link correctly in windows.

agentsofshield avatar Nov 11 '19 14:11 agentsofshield

@agentsofshield which cmake project are you referring to? is it the work-in-progress that exists on the cmake branch or are you referring to something else?

seladb avatar Nov 11 '19 22:11 seladb

@seladb yup to the cmake branch

agentsofshield avatar Nov 12 '19 09:11 agentsofshield

Windows support is not yet really working, it is still work in progress, please feel free to add that part :)

seladb avatar Nov 12 '19 09:11 seladb

hi @eteran , any progress on that? I can't wait to use cmake for this project.

DinoStray avatar Jul 08 '20 03:07 DinoStray

@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.

seladb avatar Jul 08 '20 07:07 seladb

@DinoStray @seladb

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 avatar Jul 08 '20 17:07 eteran

@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.

seladb avatar Jul 09 '20 07:07 seladb

I think we should consider separate linux and windows build. A big problem may be solved step by step,

DinoStray avatar Sep 04 '20 06:09 DinoStray

We can definitely do it, but there is a demand for both platforms 😃

seladb avatar Sep 04 '20 07:09 seladb

@eteran @DinoStray @agentsofshield

Can I have your review on the Cmake support PR ?

https://github.com/seladb/PcapPlusPlus/pull/652

clementperon avatar Jan 16 '22 16:01 clementperon