-
Notifications
You must be signed in to change notification settings - Fork 185
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
Removing libseccomp installations and disabling CGO #2063
Removing libseccomp installations and disabling CGO #2063
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.
Hi.
An external contribution which removes an action in your CI: I say YES!
I have some comments but they are minor ones, I will run the CI to see if everything builds correctly.
Best regards.
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.
Everything is OK from the CI point of view and the code addresses the problem well! Thank you!
Can you please squash your 3 commits in one, as they share the same semantic?
Signed-off-by: Amit Schendel <amits@armosec.io>
bffd058
to
2ecc9fa
Compare
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.
One minor nit. Already LGTM.
@eiffel-fl now that we don't have CGO anymore, are these libraries still needed?
inspektor-gadget/Dockerfiles/gadget-default.Dockerfile
Lines 22 to 38 in 2ecc9fa
# We need a cross compiler and libraries for TARGETARCH due to CGO. | |
RUN set -ex; \ | |
export DEBIAN_FRONTEND=noninteractive; \ | |
dpkg --add-architecture ${TARGETARCH} && \ | |
apt-get update && \ | |
apt-get install -y gcc make ca-certificates git libelf-dev:${TARGETARCH} \ | |
pkg-config:${TARGETARCH} && \ | |
if [ "${TARGETARCH}" != "${BUILDARCH}" ]; then \ | |
if [ ${TARGETARCH} = 'arm64' ]; then \ | |
apt-get install -y gcc-aarch64-linux-gnu; \ | |
elif [ ${TARGETARCH} = 'amd64' ]; then \ | |
apt-get install -y gcc-x86-64-linux-gnu; \ | |
else \ | |
>&2 echo "${TARGETARCH} is not supported"; \ | |
exit 1; \ | |
fi \ | |
fi |
@@ -108,7 +108,7 @@ registry. | |||
|
|||
For running unit tests, the following additional requirements need to be installed and configured on your system: | |||
- gcc compiler | |||
- `pkg-config` and `libseccomp-dev` libraries | |||
- `pkg-config` helper |
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.
- `pkg-config` helper | |
- `pkg-config` |
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.
The above contains "gcc compiler", so I guess having "pkg-config
helper" is OK.
Otherwise, we can just remove both "compiler" and "helper".
We should not need them, particularly as removing I will focus on |
Merging as if we remove |
[Title: Remove libseccomp and CGO usage]
Removed all installations of the package from the code.
How to use
[ describe what reviewers need to do in order to validate this PR ]
Testing done
[Describe the testing you have done before submitting this PR. Please include both the commands you issued as well as the output you got.]