Skip to content

Build system: Add WITH_COVERAGE to compile netdissect and tcpdump with code coverage #897

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fenner
Copy link
Contributor

@fenner fenner commented Jan 5, 2021

Build with --coverage and use the coveralls tool to submit
code coverage reports to https://coveralls.io/github/the-tcpdump-group/tcpdump

Support for coverage builds is added to both automake and cmake, but
coverage relies on running tests, and running tests is currently broken
with cmake. So the added job is CMAKE=no but can be changed to
CMAKE=yes when running tests with cmake is fixed.

Enabling publishing to coveralls.io requires the "the-tcpdump-group" user
to link coveralls.io to github.

…h code coverage

Build with --coverage and use the coveralls tool to submit
code coverage reports to https://coveralls.io/github/the-tcpdump-group/tcpdump

Support for coverage builds is added to both automake and cmake, but
coverage relies on running tests, and running tests is currently broken
with cmake.  So the added job is CMAKE=no but can be changed to
CMAKE=yes when running tests with cmake is fixed.
@fxlb
Copy link
Member

fxlb commented Jan 6, 2021

Enabling publishing to coveralls.io requires the "the-tcpdump-group" user
to link coveralls.io to github.

Thanks for the PR. How do you do this link?

@infrastation
Copy link
Member

You can do that by going to https://coveralls.io, clicking on the free plan, clicking on GitHub login and granting the thing access to your GH account when prompted. It will tell exactly which access it wants before you grant it. I can do that on my account if you prefer, it should result in the same effect on the GH organization.

@fxlb
Copy link
Member

fxlb commented Jan 6, 2021

I can do that on my account if you prefer, it should result in the same effect on the GH organization.

Yes, please do it.

@infrastation
Copy link
Member

This application will be able to read your organization, team membership, and private project boards.
This application will be able to read and write commit statuses (no direct code access).
This application will be able to read your private email addresses.

Authorized and enabled for the-tcpdump-group/libpcap, the-tcpdump-group/tcpdump and the-tcpdump-group/tcpslice (the latter is sometimes more convenient for testing due to its small size and short feedback loop).

@infrastation
Copy link
Member

I had restarted the Travis job and Coverall is now displaying something.

@fenner
Copy link
Contributor Author

fenner commented Jan 7, 2021 via email

@fxlb
Copy link
Member

fxlb commented Jan 7, 2021

Many error messages with the job:

profiling:/home/travis/build:Cannot create directory
profiling:/home/travis/build/the-tcpdump-group/tcpdump/print-openflow-1.3.gcda:Skip
profiling:/home/travis/build:Cannot create directory
profiling:/home/travis/build/the-tcpdump-group/tcpdump/print-openflow-1.0.gcda:Skip
profiling:/home/travis/build:Cannot create directory
profiling:/home/travis/build/the-tcpdump-group/tcpdump/print-dvmrp.gcda:Skip
profiling:/home/travis/build:Cannot create directory
profiling:/home/travis/build/the-tcpdump-group/tcpdump/parsenfsfh.gcda:Skip
profiling:/home/travis/build:Cannot create directory
profiling:/home/travis/build/the-tcpdump-group/tcpdump/ntp.gcda:Skip
[...]

@infrastation
Copy link
Member

Did the Travis-based setup work reasonably close to what was originally expected? Coveralls will most likely need to be ported to Cirrus CI, as that's where AMD64 Linux CI currently is.

@infrastation
Copy link
Member

This integration should be simpler in Cirrus CI with Coveralls-specific steps in a task of their own like Coverity Scan. Do you need my help porting it?

@fenner
Copy link
Contributor Author

fenner commented Mar 18, 2021

This integration should be simpler in Cirrus CI with Coveralls-specific steps in a task of their own like Coverity Scan. Do you need my help porting it?

I'll check it out, thanks for the reminder.

@infrastation
Copy link
Member

Here is a draft version for Cirrus CI rebased on the current master branch. It has not been tested in any way, but the difference should be obvious: instead of embedding Coveralls-specific steps into an existing generic procedure there is a separate procedure with only the required steps.

diff --git a/.cirrus.yml b/.cirrus.yml
index a9c47890..fccf9f17 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -90,3 +90,30 @@ coverity_task:
     - apt-get -qy install git curl wget ruby rubygems ruby-json # for the coverity script
     - apt list --installed 'lib*-dev'
     - ./.ci-coverity-scan-build.sh
+
+coveralls_task:
+  name: Coveralls
+  only_if: $CIRRUS_BRANCH != 'coverity_scan'
+  container:
+    cpu: 2
+    memory: 1G
+    image: ubuntu:20.04
+  env:
+    DEBIAN_FRONTEND: noninteractive
+    MAKEFLAGS: -j 2
+    CC=gcc
+  script:
+    - apt-get -qy update
+    - apt-get -qy install git autoconf make gcc
+    - apt-get -qy install flex bison libdbus-1-dev libbluetooth-dev libnl-genl-3-dev libibverbs-dev # for libpcap
+    - apt-get -qy install libssl-dev libsmi2-dev libcap-ng-dev libpcap-dev
+    - apt list --installed 'lib*-dev'
+    - pip install --user cpp-coveralls
+    - git -C .. clone --depth ${CIRRUS_CLONE_DEPTH} --branch=master --quiet ${LIBPCAP_GIT}
+    - (cd ../libpcap && REMOTE=no CMAKE=no ./build.sh)
+# Do not use build.sh because it runs "make clean" afterwards (does this break coveralls?).
+    - touch .devel configure --with-crypto=yes --enable-smb=yes
+    - CFLAGS=--coverage LDFLAGS=--coverage ./configure
+    - make -s CFLAGS=-Werror
+    - make check
+    - coveralls --gcov-options '\-lp'
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ce4fe1a2..909bd6c3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -157,6 +157,7 @@ option(WITH_CRYPTO "Build with OpenSSL/libressl libcrypto, if available" ON)
 option(WITH_CAPSICUM "Build with Capsicum security functions, if available" ON)
 option(WITH_CAP_NG "Use libcap-ng, if available" ON)
 option(ENABLE_SMB "Build with the SMB dissector" OFF)
+option(WITH_COVERAGE "Enable coverage if using gcc" OFF)
 
 #
 # String parameters.  Neither of them are set, initially; only if the
@@ -1207,6 +1208,13 @@ add_executable(tcpdump ${TCPDUMP_SOURCE_LIST_C})
 if(NOT C_ADDITIONAL_FLAGS STREQUAL "")
     set_target_properties(tcpdump PROPERTIES COMPILE_FLAGS ${C_ADDITIONAL_FLAGS})
 endif()
+if(WITH_COVERAGE AND "${CMAKE_COMPILER_IS_GNUCC}")
+    target_compile_options(tcpdump PUBLIC --coverage)
+    target_link_options(tcpdump PUBLIC --coverage)
+    target_compile_options(netdissect PUBLIC --coverage)
+    target_link_options(netdissect PUBLIC --coverage)
+endif()
+
 target_link_libraries(tcpdump netdissect ${TCPDUMP_LINK_LIBRARIES})
 
 ######################################

@fenner
Copy link
Contributor Author

fenner commented Apr 4, 2022

I've gotten it to do ... something ... from cirrus-ci - the result is at
https://coveralls.io/jobs/97271148
The job said it was pending on coveralls, even after cirrus-ci completed, and manually checking the "mark job as completed" button resulted in saying "?% coverage", even though the file browser shows files with coverage.

Getting this far required setting the repo token in the job. I set it in cirrus's web interface, clicking on the gear icon at https://cirrus-ci.com/github/fenner/tcpdump and adding COVERALLS_REPO_TOKEN to the environment, with the value of the token that is visible to me at https://coveralls.io/github/fenner/tcpdump . Conventional wisdom is that you add this as an encrypted environment variable to your repo, which I guess is OK except there's no way to allow both fenner/tcpdump and the-tcpdump-group/tcpdump to use the same config file.

I'll put this on hold again for now. The changes that I've done for this so far are at https://github.com/fenner/tcpdump/tree/coveralls-update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants