Fix build failures on MinGW during package CI #1452
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1452 +/- ##
=======================================
Coverage 82.53% 82.53%
=======================================
Files 171 171
Lines 22399 22399
Branches 8548 8548
=======================================
Hits 18487 18487
+ Misses 3114 3108 -6
- Partials 798 804 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -621,7 +621,6 @@ namespace pcpp | |||
| std::vector<uint8_t> Asn1IntegerRecord::encodeValue() const | |||
| { | |||
| std::vector<uint8_t> result; | |||
| result.reserve(m_ValueLength); | |||
There was a problem hiding this comment.
I have no idea why the compiler complains here, but I think it's fine to remove it:
https://github.com/seladb/PcapPlusPlus/actions/runs/9562329238/job/26358481698
There was a problem hiding this comment.
can we ignore this line only for MinGW?
There was a problem hiding this comment.
It's a compilation error so not sure we can ignore it, but I think removing it is ok, I don't foresee a big performance impact because of it
There was a problem hiding this comment.
I mean ifndef MinGW or some similar syntax.
I think it's better to keep the reserve as it still works for other platforms.
There was a problem hiding this comment.
I tried with __MIGW32__ / __MINGW64__, but it didn't work: https://github.com/seladb/PcapPlusPlus/actions/runs/9593015503/job/26452660814
Then I changed to __MINGW32_VERSION_MAJOR / __MINGW64_VERSION_MAJOR as we do here and it worked: https://github.com/seladb/PcapPlusPlus/actions/runs/9593255865
seladb
changed the title
|
@seladb last thing before merging, could you add a note for every place you made changes? |
This PR addresses this issue: #1347
I'm not entirely sure how the MinGW build we have in the
packageworkflow is different from the one we have in thebuild_and_testworkflow (both have the same versions and are running on the same type of VM), but anyway I addressed the build failure and they all seem to pass now:package: https://github.com/seladb/PcapPlusPlus/actions/runs/9577752848build_and_test: https://github.com/seladb/PcapPlusPlus/actions/runs/9577752845