-
Notifications
You must be signed in to change notification settings - Fork 510
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
remove the libbzip2 package #3944
Conversation
Our libelf and libdw are not built with bzip2 support, so there is no need to link the makedumpfile binary with libbz2. Signed-off-by: Ben Cressey <[email protected]>
Bottlerocket's modules are only a few hundred KiB, versus several MiB for the "targeted" reference policy, so compression doesn't save very much space. Signed-off-by: Ben Cressey <[email protected]>
Patch out support for reading and writing bzip2-compressed modules, to avoid the otherwise-unneeded dependency on libbz2. Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
@@ -64,7 +64,7 @@ install -d "${moddir}" | |||
for m in *.cil ; do | |||
mod="${m%.*}" | |||
install -d "${moddir}/${mod}" | |||
bzip2 -c "${m}" > "${moddir}/${mod}/cil" | |||
install -p -m 0644 "${m}" "${moddir}/${mod}/cil" |
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.
Will we consider using ZSTD?
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.
Will we consider using ZSTD?
I'd like to, but probably only if it ends up as an upstream feature. The benefits from compression here are so modest that I don't think it's worth carrying an even larger patch to implement that.
#include <unistd.h> | ||
#include <fcntl.h> | ||
|
||
-#include <bzlib.h> |
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.
I wonder if upstream will be interested in a patch that includes conditionals for compressed modules. Are you aware of any discussion about other compression libraries?
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.
I haven't seen any recent discussion; in past threads, compression tended to be challenged with "is this really necessary?" which strikes me as totally correct and fair.
If we really needed compression, we could work around this in Bottlerocket with a combination of squashfs/erofs (as we do with licenses and kernel devel files) and overlayfs (to add a writable upper layer). But that seems like overkill here.
@@ -1,19 +0,0 @@ | |||
[package] | |||
name = "libbzip2" |
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.
Lovely! One less package to update!
Issue number:
N/A
Description of changes:
In the wake of the xz event, I'd like to continue removing older, rarely updated compression libraries. bzip2 is the obvious next candidate since it's only depended on by two packages.
In the case of
makedumpfile
it's not actually even required; it's just specified in the default link flags under the assumption thatlibelf
andlibdw
will depend on that library.Removing the
libsemanage
dependency is a bit more involved. The resulting patch is not upstreamable in this form, but suffices for Bottlerocket with its tiny SELinux policy files. It shouldn't be difficult to maintain given thatcompressed_file.c
hasn't changed in the past two years.Testing done:
Verified that
semodule
works with the uncompressed files:Verified that
makedumpfile
works:Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.