Skip to content

userspace: Make Cflags in our .pc files more strict#2691

Merged
poiana merged 2 commits intofalcosecurity:masterfrom
geraldcombs:pkg-config-strict-includes
Oct 28, 2025
Merged

userspace: Make Cflags in our .pc files more strict#2691
poiana merged 2 commits intofalcosecurity:masterfrom
geraldcombs:pkg-config-strict-includes

Conversation

@geraldcombs
Copy link
Copy Markdown
Contributor

@geraldcombs geraldcombs commented Oct 14, 2025

Remove "-I${includedir}/libscap -I${includedir}/driver" from libscap.pc and libsinsp.pc. This means that consumers must use prefixed include paths, such as "#include <libscap/scap.h>" instead of "#include <scap.h>". However, those include directories contain several generic filenames such as settings.h, logger.h, plugin.h, user.h, and utils.h, and this reduces the risk of someone inadvertently including the wrong file.

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

This hopefully helps us avoid a situation like the following: https://gitlab.com/wireshark/wireshark/-/issues/17192

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@poiana poiana requested review from hbrueckner and terror96 October 14, 2025 21:56
@geraldcombs
Copy link
Copy Markdown
Contributor Author

geraldcombs commented Oct 14, 2025

Hi @apteryks, this partially reverts 0ef229d. Will that impact anything on your end?

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 14, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.91%. Comparing base (0376a8f) to head (aeff7a0).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2691   +/-   ##
=======================================
  Coverage   76.91%   76.91%           
=======================================
  Files         294      294           
  Lines       30862    30862           
  Branches     4689     4689           
=======================================
  Hits        23739    23739           
  Misses       7123     7123
Flag Coverage Δ
libsinsp 76.91% <ø> (ø)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@geraldcombs geraldcombs force-pushed the pkg-config-strict-includes branch 2 times, most recently from bb8d87c to 46cd00d Compare October 14, 2025 23:53
@apteryks
Copy link
Copy Markdown
Contributor

apteryks commented Oct 15, 2025

Hi @apteryks, this partially reverts 0ef229d. Will that impact anything on your end?

$ LANG=C guix refresh --list-dependent falcosecurity-libs
A single dependent package: sysdig@0.40.1

If it breaks anything, that'd be sysdig. Has it been adjusted for this change already?

@geraldcombs
Copy link
Copy Markdown
Contributor Author

geraldcombs commented Oct 15, 2025

If it breaks anything, that'd be sysdig. Has it been adjusted for this change already?

Thanks for catching that. Corresponding PR in draios/sysdig#2166.

@terror96
Copy link
Copy Markdown
Contributor

terror96 commented Oct 16, 2025

/milestone 0.23.0

@poiana poiana added this to the 0.23.0 milestone Oct 16, 2025
@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented Oct 23, 2025

Hey @geraldcombs , thank you for the contribution! Changes look good, but could you please adjust the commit heading to match the conventional commit style? I guess it would be something like refactor(userspace): make cflags in our .pc files more strict, right?

@geraldcombs geraldcombs force-pushed the pkg-config-strict-includes branch from 46cd00d to 0336274 Compare October 23, 2025 15:54
@geraldcombs
Copy link
Copy Markdown
Contributor Author

geraldcombs commented Oct 23, 2025

I guess it would be something like refactor(userspace): make cflags in our .pc files more strict, right?

Done!

@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented Oct 24, 2025

Sorry, but I guess we need a rebase on top the changes introduced by #2702. Same here: #2692

@geraldcombs
refactor(userspace): Make Cflags in our .pc files more strict
be37a1c 
Remove "-I${includedir}/libscap -I${includedir}/driver" from libscap.pc
and libsinsp.pc. This means that consumers must use prefixed include
paths, such as "#include <libscap/scap.h>" instead of "#include
<scap.h>". However, those include directories contain several generic
filenames such as settings.h, logger.h, plugin.h, user.h, and utils.h,
and this reduces the risk of someone inadvertently including the wrong
file.

Signed-off-by: Gerald Combs <gerald@wireshark.org>
@geraldcombs geraldcombs force-pushed the pkg-config-strict-includes branch from 0336274 to be37a1c Compare October 24, 2025 14:57
@geraldcombs
update(userspace): Add "License" properties to our .pc files
aeff7a0 
pkgconf supports a License property, which seems like a useful thing to
have.

Signed-off-by: Gerald Combs <gerald@wireshark.org>
@poiana poiana added size/S and removed size/XS labels Oct 24, 2025
ekoops
ekoops approved these changes Oct 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ekoops ekoops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@poiana poiana added the lgtm label Oct 27, 2025
@poiana
Copy link
Copy Markdown
Contributor

poiana commented Oct 27, 2025

LGTM label has been added.

DetailsGit tree hash: 81c2031fe945303180385d5f5269e41204915927

terror96
terror96 approved these changes Oct 28, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@terror96 terror96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@github-project-automation github-project-automation bot moved this from Todo to In progress in Falco Roadmap Oct 28, 2025
@poiana poiana merged commit 04eaffe into falcosecurity:master Oct 28, 2025
46 of 47 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Falco Roadmap Oct 28, 2025
@poiana
Copy link
Copy Markdown
Contributor

poiana commented Oct 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ekoops, geraldcombs, terror96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@BrewTestBot BrewTestBot mentioned this pull request Dec 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terror96 terror96 terror96 approved these changes

@ekoops ekoops ekoops approved these changes

@hbrueckner hbrueckner Awaiting requested review from hbrueckner

Assignees

@terror96 terror96

@ekoops ekoops

Labels

approved area/build dco-signoff: yes kind/cleanup lgtm release-note-none size/S

Projects

Falco Roadmap
Archived in project

Milestone

0.23.0

Development

Successfully merging this pull request may close these issues.

5 participants

@geraldcombs @apteryks @terror96 @ekoops @poiana