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

fix(build): static build might require zstd lib #4059

Merged
merged 3 commits into from
May 21, 2024

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented May 21, 2024

Close: #4060

1. Explain what the PR does

ac9bdb1 chore: temp fix for install libzstd on AMIs
656df9a chore: install required bpftool from btfhub
e40109f fix(build): static build might require zstd lib

ac9bdb1 chore: temp fix for install libzstd on AMIs

AMIs have to be updated and installations like this commented out.

e40109f fix(build): static build might require zstd lib

libelf might be built with zstd support in some environments, so for
static builds (mainly) it is necessary to ensure that the zstd library
is available.

https://sourceware.org/git/?p=elfutils.git;a=commit;h=ed688a59b4d4f5ccf6ef15244e5a9139f71769a3

2. Explain how to test it

3. Other comments

libelf might be built with zstd support in some environments, so for
static builds (mainly) it is necessary to ensure that the zstd library
is available.

https://sourceware.org/git/?p=elfutils.git;a=commit;h=ed688a59b4d4f5ccf6ef15244e5a9139f71769a3
@geyslan
Copy link
Member Author

geyslan commented May 21, 2024

This would make room for aquasecurity/btfhub#117.

@geyslan geyslan requested a review from rscampos May 21, 2024 00:24
@geyslan
Copy link
Member Author

geyslan commented May 21, 2024

https://github.com/aquasecurity/tracee/actions/runs/9166787615/job/25202867174?pr=4059

e2e-install-deps.sh must have (for now) the installation of zstd.

@geyslan geyslan marked this pull request as draft May 21, 2024 00:34
AMIs have to be updated and installations like this commented out.
@geyslan geyslan force-pushed the fix-static-build branch from 789fd7f to ac9bdb1 Compare May 21, 2024 19:02
@geyslan geyslan marked this pull request as ready for review May 21, 2024 19:09
Comment on lines +267 to +282
install_libzstd_os_packages() {
case $ID in
"ubuntu")
wait_for_apt_locks
apt-get install -y libzstd-dev
;;
"almalinux")
yum install -y libzstd-devel
;;
*)
echo "Unsupported OS: $ID"
exit 1
;;
esac
}

Copy link
Member Author

@geyslan geyslan May 21, 2024

Choose a reason for hiding this comment

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

All this would be unnecessary if we opted for an easier way of updating the AMIs bunch in a single shot. Anyway, after its future upgrade, those steps should be commented/removed.

@@ -95,7 +95,7 @@ Vagrant.configure("2") do |config|
ln -s "$path" "${path%-*}"
done

apt-get install --yes zlib1g-dev libelf-dev
apt-get install --yes zlib1g-dev libelf-dev libzstd-dev
Copy link
Member Author

Choose a reason for hiding this comment

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

As we discussed @rscampos, the setup of the bpftool in Vagrantfile must be done in a further PR (since it requires a system upgrade). Thanks for noticing that. 👍🏼

Copy link
Collaborator

@rscampos rscampos May 21, 2024

Choose a reason for hiding this comment

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

I'll upgrade Vagrantfile in order use compiled version of bpftool :)

Copy link
Collaborator

@rscampos rscampos left a comment

Choose a reason for hiding this comment

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

LGTM

@geyslan geyslan merged commit ba4db8b into aquasecurity:main May 21, 2024
32 checks passed
@geyslan geyslan deleted the fix-static-build branch June 28, 2024 18:26
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.

Static build can be broken in some environments
2 participants