-
Notifications
You must be signed in to change notification settings - Fork 170
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: upgrade deps and cmakefiles improvements #30
Conversation
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.
Lots of changes here, I hoped for a bit more explanation :)
278e3fe
to
24e5613
Compare
Update: I'm re-working this PR again |
ce60c9a
to
86a473a
Compare
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
This commit also introduces other relevant changes: - now using gRPC cmakefiles (that requires a workaround, see the comment) - added support for gpr lib (likely needed by dependant, for example Falco) - switched from `_unsecure` grpc libs to default ones Signed-off-by: Leonardo Grasso <[email protected]>
… small improvements Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
The deprecation annotation produced warnings when building. Since the warning is not really relevant to libs code and might be problematic for dependant projects (especially when a project uses warnings as errors, like Falco), we are just suppressing it by removing the annotation. Co-authored-by: Grzegorz Nosek <[email protected]> Signed-off-by: Leonardo Grasso <[email protected]>
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
I've tried out various combinations of builds.
Note:
when using make
as a generator it can happen that (it seems because of OpenSSL) sinsp
doesn't build at the first attempt, not able to reproduce this with ninja.
LGTM label has been added. Git tree hash: 0c06397fb26ab3a437d6a0d858d3b24e62855d82
|
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area build
What this PR does / why we need it:
This PR upgrades c-ares, protobuf and gRPC versions. It also changes the way we build gRPC when deps are bundled. The building of protobuf and c-ares is not delegated anymore to gRPC.
This PR also includes other changes (some required by the gRPC upgrade, some are just small improvements):
_usecure
libs to default onesno-shared
--disable-shared
Finally, other changes are included to easier the libraries integration into the Falco build system, see also falcosecurity/falco#1552
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Unfortunately, this PR introduces a workaround, see https://github.com/falcosecurity/libs/pull/30/files#diff-80c09a7dee6a7fbc4e1648084e71598c7e28978e00ff50ab9381e7b8e20ec8b8R49
Since this PR took longer than expected and is blocking falcosecurity/falco#1552 I'd prefer to fix that later.
Does this PR introduce a user-facing change?: