-
Notifications
You must be signed in to change notification settings - Fork 715
Providing DEB #1831
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
base: dev
Are you sure you want to change the base?
Providing DEB #1831
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1831 +/- ##
==========================================
- Coverage 83.00% 82.99% -0.01%
==========================================
Files 284 285 +1
Lines 49006 49012 +6
Branches 10574 10343 -231
==========================================
+ Hits 40676 40679 +3
- Misses 7215 7216 +1
- Partials 1115 1117 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
.github/workflows/package.yml
Outdated
apt-get update | ||
apt-get install -y --no-install-recommends cmake make g++ libpcap-dev |
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.
This step fails because we don't run it as sudo
, which made me notice that this runs directly on the GitHub runner VM. Why not run it inside one of our containers like seladb/ubuntu2404
? This image should already have these dependencies pre-installed
.github/workflows/package.yml
Outdated
cd "${{ env.BUILD_DIR }}" | ||
cmake .. | ||
make -j$(nproc) | ||
make install DESTDIR=$PWD/install-root |
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.
Instead of installing PcapPlusPlus here, maybe we can build the deb
file, install it, and add another step to test that it works?
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.
Looking into it
.github/workflows/package.yml
Outdated
Version: ${{ github.event.release.tag_name }} | ||
Section: libs | ||
Architecture: amd64 | ||
Maintainer: Bhaskar Bhar <bhaskarbhar007@yahoo.com> |
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.
Can we add pcapplusplus@gmail.com
as a maintainer?
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.
Yes sir
.github/workflows/package.yml
Outdated
- name: Upload DEB to release | ||
uses: ncipollo/release-action@440c8c1cb0ed28b9f43e4d1d670870f059653174 # v1.16.0 | ||
with: | ||
draft: true | ||
allowUpdates: true | ||
updateOnlyUnreleased: true | ||
artifacts: "${{ env.BUILD_DIR }}/pcapplusplus_${{ github.event.release.tag_name }}_amd64.deb" |
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.
Should this not have an if
clause? The rest of the package CI's have the upload steps under if: github.ref_type == 'tag'
clause?
@bhaskarbhar No worries about the flaws. That is why the reviews are for. Ty for the effort. :) |
@bhaskarbhar why did you close the PR? 🤔 |
As there were many flaws I thought to go through the package.yml and correct it rather than commiting everytime. Wanted to do a fresh PR with no flaws |
@Dimi1010 Removed Test, corrected indentation. The build-deb check in Package and Release are passed. |
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.
LGTM
@clementperon @egecetin @tigercosmos I'm not an expert in this domain, maybe you can also take a look? |
@seladb Cmake can handle deb generation. I think this MR is a bit complicated and could be simplified using cmake deb packaging no ? |
Looking into it. |
I found this in CmakeLists.txt:
According to you suggestion, I will remove the comment from CmakeLists and rebuild the build-deb job. |
Exactly then maybe the actual package pipeline need some rework but not much I think. Also there should be two deb generated. |
Dev and runtime??? Two deb?? I need clarification in this. I have only updated the CmakeLists and package.yml for now. Please guide me as I am unable to understand what are the requirements now. |
I think that's why I didn't enable it yet.
@seladb @Dimi1010 at the moment only uncomment the # CPACK_DEB will generate also bin, cmake and .h. You can find .deb example here: https://github.com/clementperon/PcapPlusPlus/releases/tag/25.06
|
@clementperon Will the original issue change now (Issue: #1498)? As both dev and runtime are required. I created this https://github.com/bhaskarbhar/PcapPlusPlus/releases/download/v25.05-deb/pcapplusplus_25.05_amd64.deb at the very beginning. I created this using the previous methods before you suggested usig cmake -G. I guess its now more complex for me as I am just a beginner in open source :(( , but I will try meeting the requirements. For now can you review the latest build-deb job in package.yml. |
@bhaskarbhar I have pushed on my repo an example but don't have time to properly test everything right now. |
Understood. Also the
|
Added a new build-deb job to build a Debian package (.deb) for PcapPlusPlus on ubuntu-latest.
The job:
Issue: #1498