Skip to content
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

build: reorganize driver cmake vars #423

Merged
merged 5 commits into from
Jun 23, 2022

Conversation

leogr
Copy link
Member

@leogr leogr commented Jun 22, 2022

What type of PR is this?

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

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

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

/area build

/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-udig

/area libscap

/area libsinsp

/area tests

/area proposals

What this PR does / why we need it:

This is mainly required to allow Falco to include different versions of libs and drivers. Falco can now pull libs WITH_DRIVER=Off and separately pull the driver. Without this patch, the driver target was always added.
Some other minor improvements are included.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

leogr added 3 commits June 22, 2022 14:59
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
…the driver from the build

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
@FedeDP
Copy link
Contributor

FedeDP commented Jun 22, 2022

I really like this refinement!
Thank you Leo!

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
@jasondellaluce
Copy link
Contributor

I think this is great Leo!

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
set(DRIVER_NAME "scap")
endif()

if(NOT DEFINED DRIVER_DEVICE_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be used by scap too:

snprintf(filename, sizeof(filename), "%s/dev/" DRIVER_DEVICE_NAME "%d", scap_get_host_root(), all_scanned_devs);

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, but it's not an issue. Since it's defined in driver_config.h which is autogenerated and included 👇

#include "driver_config.h"

All others definition in driver_config.h work in the same way.

Am I wrong?

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

Copy link
Contributor

@FedeDP FedeDP 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
Copy link
Contributor

poiana commented Jun 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, leogr

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:
  • OWNERS [Andreagit97,FedeDP,leogr]

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

@poiana poiana merged commit 3911e98 into master Jun 23, 2022
@poiana poiana deleted the build/reorganize-driver-cmake-vars branch June 23, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants