Force pcapplusplus prefix in include

[clementperon]

At the moment building a simple example like

CMakeLists.txt

      project(TestPcapPlusPlus)
      set(CMAKE_CXX_STANDARD 11)
      find_package(PcapPlusPlus CONFIG REQUIRED)
      add_executable(test test.cpp)
      target_link_libraries(test PUBLIC PcapPlusPlus::Pcap++)
      set_target_properties(test PROPERTIES NO_SYSTEM_FROM_IMPORTED ON)

test.cpp

      #include <cstdlib>
      #include <pcapplusplus/PcapLiveDeviceList.h>
      int main() {
        const std::vector<pcpp::PcapLiveDevice*>& devList =
          pcpp::PcapLiveDeviceList::getInstance().getPcapLiveDevicesList();
        if (devList.size() > 0) {
          if (devList[0]->getName() == "")
            return 1;
          return 0;
        }
        return 0;
      }

Trig an error on MacOS when PcapPlusplus is installed with brew

VERBOSE=1 cmake --build build
[ 50%] Building CXX object CMakeFiles/test.dir/test.cpp.o
/Library/Developer/CommandLineTools/usr/bin/c++  -I/opt/homebrew/include/pcapplusplus -std=gnu++11 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk -MD -MT CMakeFiles/test.dir/test.cpp.o -MF CMakeFiles/test.dir/test.cpp.o.d -o CMakeFiles/test.dir/test.cpp.o -c /Users/clement/work/test-pcappp/test.cpp
/Users/clement/work/test-pcappp/test.cpp:2:16: fatal error: 'pcapplusplus/PcapLiveDeviceList.h' file not found
    2 |       #include <pcapplusplus/PcapLiveDeviceList.h>
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/test.dir/test.cpp.o] Error 1
make[1]: *** [CMakeFiles/test.dir/all] Error 2
make: *** [all] Error 2

This is because the include folder is " -I/opt/homebrew/include/pcapplusplus"
and file is located in /opt/homebrew/include/pcapplusplus/PcapLiveDeviceList.h

Remove the pcapplusplus folder

[Dimi1010]

Any reason this merges in the master branch?

Nvm, the change of base branch was not updated for me.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.11%. Comparing base (9130bf1) to head (9d7bfd1).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1586      +/-   ##
==========================================
+ Coverage   83.10%   83.11%   +0.01%     
==========================================
  Files         276      276              
  Lines       46889    46907      +18     
  Branches     9365     9587     +222     
==========================================
+ Hits        38965    38989      +24     
- Misses       7065     7067       +2     
+ Partials      859      851       -8     

Flag	Coverage Δ
fedora40	74.71% <100.00%> (+0.03%)	⬆️
macos-12	81.10% <50.00%> (ø)
macos-13	80.53% <50.00%> (+0.07%)	⬆️
macos-14	80.44% <50.00%> (-0.01%)	⬇️
mingw32	70.61% <100.00%> (-0.03%)	⬇️
mingw64	70.57% <100.00%> (-0.03%)	⬇️
npcap	84.99% <ø> (-0.03%)	⬇️
rhel94	74.56% <100.00%> (+<0.01%)	⬆️
ubuntu2004	57.93% <100.00%> (ø)
ubuntu2004-zstd	58.05% <100.00%> (ø)
ubuntu2204	74.48% <100.00%> (-0.03%)	⬇️
ubuntu2204-icpx	58.70% <50.00%> (ø)
ubuntu2404	74.74% <100.00%> (-0.01%)	⬇️
unittest	83.11% <100.00%> (+0.01%)	⬆️
windows-2019	85.03% <ø> (-0.03%)	⬇️
windows-2022	85.04% <ø> (-0.03%)	⬇️
winpcap	85.02% <ø> (ø)
xdp	49.65% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[seladb]

@clementperon I remember we discussed it, but I forgot why we need this change. Where does homebrew expect the header files to be, and where are they now?

[clementperon]

@clementperon I remember we discussed it, but I forgot why we need this change. Where does homebrew expect the header files to be, and where are they now?

@seladb

Homebrew will install the files in "/opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus/xxx.h"

The actual CMake will provide the following include directory

• "/opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus"

This MR make the proper change to give this directory

• "/opt/homebrew/Cellar/pcapplusplus/24.09/include"

We could also keep a fallback compatibility by including both folder for the moment:

• "/opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus"
• "/opt/homebrew/Cellar/pcapplusplus/24.09/include"

But it's proper to use "pcapplusplus/xxxx.h" everywhere in our code no ?

[seladb]

@clementperon Here are my thoughts:

• This is a major breaking change because any project that uses PcapPlusPlus would need to adjust their #include. A solution could be placing the header files in both /opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus and /opt/homebrew/Cellar/pcapplusplus/24.09/include, but it's not very nice...
• I think that PcapPlusPlus code doesn't need to change and should still #include "xxx.h". Only the tests and examples should use the new #include pcapplusplus/xxx.h
• As we previously discussed I don't think the new structure of having pcappplusplus/include in Common++, Packet++ and Pcap++ is very nice

I'm wondering if it's worth the change, or what are the other options we have 🤔

[clementperon]

@clementperon Here are my thoughts:

• This is a major breaking change because any project that uses PcapPlusPlus would need to adjust their #include. A solution could be placing the header files in both /opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus and /opt/homebrew/Cellar/pcapplusplus/24.09/include, but it's not very nice...

To me the final user should always use

#include <pcapplusplus/XXXX.h>

Which is at the moment broken on MacOS and can also break on Linux depends where you install PcapPlusPlus.
On linux it mostly works because the header are installed in /usr/local/include and /usr/local/include is a default path.

If we want to keep the compatibilty with

#include "XXXX.h"

Then we can include both folder in the CMake and it will work without breaking nobody.

• I think that PcapPlusPlus code doesn't need to change and should still #include "xxx.h". Only the tests and examples should use the new #include pcapplusplus/xxx.h

I agree, but tests and examples are libraries that have the possibility to be built in the pcapplusplus project.
I could fix that, if we don't link with library like we do at the moment.
see:
https://github.com/seladb/PcapPlusPlus/blob/master/Examples/CMakeLists.txt#L16C15-L16C27

• As we previously discussed I don't think the new structure of having pcappplusplus/include in Common++, Packet++ and Pcap++ is very nice

I'm wondering if it's worth the change, or what are the other options we have 🤔

[seladb]

@clementperon Here are my thoughts:

* This is a major breaking change because any project that uses PcapPlusPlus would need to adjust their `#include`. A solution could be placing the header files in both `/opt/homebrew/Cellar/pcapplusplus/24.09/include/pcapplusplus` and `/opt/homebrew/Cellar/pcapplusplus/24.09/include`, but it's not very nice...

* I think that PcapPlusPlus code doesn't need to change and should still `#include "xxx.h"`. Only the tests and examples should use the new `#include pcapplusplus/xxx.h`

* As we previously discussed I don't think the new structure of having `pcappplusplus/include` in `Common++`, `Packet++` and `Pcap++` is very nice

I'm wondering if it's worth the change, or what are the other options we have 🤔

Let's add more people to the discussion to hear their thoughts: @tigercosmos @egecetin @Dimi1010 , what do you think?