-
Notifications
You must be signed in to change notification settings - Fork 167
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
fix(userspace/libscap): scap-gvisor does need to depend upon jsoncpp. #2112
Conversation
Signed-off-by: Federico Di Pierro <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/milestone 0.19.0 |
Perf diff from master - unit tests
Heap diff from master - unit tests
Heap diff from master - scap file
Benchmarks diff from master
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2112 +/- ##
=======================================
Coverage 73.69% 73.69%
=======================================
Files 253 253
Lines 31914 31914
Branches 5627 5619 -8
=======================================
Hits 23519 23519
+ Misses 8375 8374 -1
- Partials 20 21 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
X64 kernel testing matrix
ARM64 kernel testing matrix
|
cc @gnosek |
@@ -56,15 +56,11 @@ add_custom_command( | |||
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | |||
) | |||
|
|||
if(USE_BUNDLED_JSONCPP) | |||
add_dependencies(scap jsoncpp) |
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.
Note scap
instead of scap_engine_gvisor
add_library( | ||
scap_engine_gvisor ${scap_engine_gvisor_sources} ${scap_engine_gvisor_generated_sources} | ||
) | ||
|
||
add_dependencies(scap_engine_gvisor uthash) | ||
add_dependencies(scap_engine_gvisor uthash jsoncpp) |
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.
No need to
if(USE_BUNDLED_JSONCPP)
since the jsoncpp cmake module always creates the jsoncpp
target.
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area libscap-engine-gvisor
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Since gvisor sources use jsoncpp, it must depend upon it. Most probably that was an unintentional typo.
See https://github.com/falcosecurity/falco/actions/runs/11324106507/job/31488703414?pr=3380 for a failing CI, where scap_engine_gvisor gets build before jsoncpp and therefore gvisor cannot find
json/json.h
include.This is something purely time-based, ie: i am pretty sure that if we compile with a different
-jX
it would pass the build.I think the time issue got somewhat exacerbated by #2005 changes.
In the end:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: