-
Notifications
You must be signed in to change notification settings - Fork 36
add winpcap/4.1.2 #1395
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
add winpcap/4.1.2 #1395
Conversation
I'm not sure why it fails in CI on VS 2017/2019. |
The build is still failing. Any idea why? It works well on my local machine... |
No idea. Maybe you can think about a potential difference between your local machine and the CI environment? |
@Croydon I figured out what the problem was: when running something that needs WinPcap you need |
Good to know what the problem is, but adding libraries directly to the test_package is not a solution Why are those libraries only missing on Windows? Should they be present in this Conan package? |
WinPcap is only available for Windows. For other platforms libpcap should be used.
This Conan package wraps the WinPcap SDK. It doesn't run the WinPcap installer as I don't think Conan should install anything on the user's system (please correct me if I'm wrong). Adding the Please let me know what you think |
@Croydon can you please advise on how we can move forward with this PR? |
I now remember this nightmare. Yes, indeed, this was the biggest thing that made Let me review the landscape again and consider what the best course of action is. I will say one thing right now for sure... there is no simple solution in this case. There are a few options, and each comes with significant implications and sacrifices. |
Thanks @solvingj , I really appreciate your help with this! |
Sorry, for the late response. I don't know much about winpcap, but if it is possible to build wpcap.dll and packet.dll from source, it might be best to add those to this package? |
hi @Croydon I'm not sure that makes a lot of sense because this package installs the WinPcap SDK without building it from source, so building the WinPcap DLLs (which are usually installed from an installer file: https://www.winpcap.org/install/bin/WinPcap_4_1_3.exe) will probably not be a good idea. @solvingj did you have a chance to take a look at this? |
So, I completely agree with @Croydon in theory.
However... What's wrong with just building from source? Well, the reality is that the total complexity involved with re-creating these two All of this could have been fixed upstream, but sadly the WinPcap project got acquired by a commercial company, and then abandoned... basically freezing it forever in a state of horror. The situation is so bad, that it actually MIGHT justify making an installer wrapper package and treating like a But wait... there's hope! We can consider an alternative. As with most popular-but-abandoned OSS projects, someone picks up the baton. The brilliant security folks at nmap.org created an alternative packet driver for Windows called But.... alas... more pain. Even this is not the silver-bullet it could have been. The
In summary, Sadly, despite all this history, I do not have good advice for you (@seladb) or us (bincrafters) on how to proceed with this. The only thing that would seemingly make it acceptable for me is a complete backwards-compatible clean-room re-write of WinPCAP with a traditional open-source license model which will never happen. @Croydon given the complications with building the .dll from source, what are your thoughts? |
@solvingj thanks for your detailed response! WinPcap is indeed very hacky and difficult to build, which is why I suggested using the pre-compiled binaries (for both the I'm not sure how we can take this forward. If you decide not to port WinPcap to the new bincrafters repo (and eventually to ConanCenter) then maybe I should consider building a PcapPlusPlus package for vcpkg which already has a WinPcap package. Let me know what you think |
If we are allowed to redistribute the official binaries of the library files then we can add them in a Bincrafters Conan recipe. At Bincrafters, we can be more flexible in what we allow and what we do not allow. That said, we always try to eventually push recipes to CCI and redistribution of binaries will likely be rejected there. However, at that future point in time, there might still be the possibility to convert it to a system package (someone could add winpcap to winget). Does this |
Unfortunately the installer doesn't come with a silent install and that's why I use this version of WinPcap installer in PcapPlusPlus CI. I've been thinking more about this and was looking at WinPcap recipes from both Bincrafters and vcpkg. Both patch the solution/project files to make the build pass. Maybe we can just use this? We can also build the |
Hello, Would anybody of you please have a look at conan-io/conan-center-index#6348? On Windows with Visual Studio, it needs the Windows Driver Kit. Perhaps you have more luck? |
@seladb |
Yes, sure! |
I think @solvingj managed to build WinPcap a few years back, maybe he can help |
@seladb Coming back to this PR after a long time... The CCI PR got closed and no one else tried so far. What do you want to do with this PR? It doesn't match today's Conan standards anymore, but there is no regression to the current winpcap recipe we have, so I'm willing to merge this. (Though further modernization are obvious welcome) |
cmake.build() | ||
|
||
def test(self): | ||
for file_name in ["1_packet.pcap", "wpcap.dll", "Packet.dll"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding binary files is pretty bad. I think the old test_package might be better.
A test_package only needs to check if the Conan package was created correctly, not test internal logic
description = "The WinPcap packet capture library." | ||
homepage = "https://www.winpcap.org/" | ||
url = "https://github.com/bincrafters/community" | ||
license = "Muliple" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that this is coming from the current Bincrafters recipe, but what are those licenses? We should list them here
license = ("x1", "x2", "x3")
@Croydon I can address these comments, but maybe it's better to open a new PR for WinPcap/4.1.2 in conan-center-index? Or do you think there's no chance they will be willing to merge it? This package will be more useful in |
Sure, if you can put in the work to get it in CCI then it will be better. If that does not work we can still merge it in BIncrafters. CCI has stricter policies in some areas. |
Description
Add WinPcap/4.1.2 recipe. Please note this is the latest stable version of WinPcap and not the dev version that was previously built from source here: https://github.com/bincrafters/conan-winpcap
This package should be good enough for all Windows users that need WinPcap (since WinPcap is no longer maintained anyway).
Related Issue
N/A
Motivation and Context
I hope this is the first step of migrating https://github.com/bincrafters/conan-winpcap to ConanCenter
How Has This Been Tested?
I wrote a brand new
test_package.cpp
that reads a packet from a pcap file to make sure linking with WinPcap works