-
Notifications
You must be signed in to change notification settings - Fork 75
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
Heroku-24: Remove unnecessary headers from build image #296
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edmorley
changed the title
Heroku-24: Remove unnecessary packages from build image
Heroku-24: Remove unnecessary headers from build image
Apr 29, 2024
joshwlewis
approved these changes
Apr 30, 2024
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.
Seems ok to me. Paging @dzuelke for a second opinion.
dzuelke
reviewed
Apr 30, 2024
A number of development packages currently being installed in the build image are actually for use-case that either do not make sense in a container context, or are not used by the dependencies of the app itself (we only need libs for app dependencies, not for the compiling runtimes themselves use-case). Many of these packages have been simply been copy-pasted from one new stack to the next, even though they are not actually needed in the image. As such, the following have been removed from the build image: - `libacl1-dev`: Used for the management of POSIX Access Control Lists. Whilst many Linux system tools use `libacl` (including `coreutils`), app dependencies won't be typically linking against it, so we don't need the headers for it. - `libapt-pkg-dev`: Development files for APT's `libapt-pkg` and `libapt-inst`. Neither our buildpacks nor apps need to link against `libapt` since they use the `apt` CLI instead. - `libaudit-dev`: For Linux Audit, whose `libaudit` is used by things like the `login` command, but otherwise not typically needed for app dependencies to link against. - `libcap-dev`: For management of POSIX 1003.1e capabilities, an alternative to the superuser model of privilege under Linux. Not something needed for app dependencies to link against. - `libkeyutils-dev`: For the management of keys in the kernel. Not useful in a non-root container context. - `libkmod-dev`: For the management of Linux Kernel modules. Not useful in a non-root container context. Also, the `libkmod` lib is not present in the run image, so this wouldn't be usable at runtime anyway. - `libpopt-dev`: For parsing cmdline parameters. Whilst many Linux system tools use `libpopt`, app dependencies won't be typically linking against it, so we don't need the headers for it. - `libreadline-dev`: Provides functionality for entering text in interactive scenarios, and the management of terminal history. Whilst many tools/runtimes will use the library, application dependencies themselves won't typically link against it, so we don't need the headers for it in the build image. (See note below about compiling runtimes.) - `libseccomp-dev`: Provides a high level interface to Linux seccomp filter. Was added in #105 to support a demo of an internal project that has since been abandoned (see heroku/build-team#83). IMO this demo use-case should instead have used the APT buildpack instead of requesting addition to the base image while experimenting. - `libsemanage-dev`: For SELinux policy manipulation. This is not something app dependencies typically ever need to link against. - `libsystemd-dev`: Development files for SystemD. Not typically needed in a container context, plus not something app dependencies typically ever need to link against. Also, the `libsystemd0` lib is not present in the run image, so this wouldn't be usable at runtime anyway. - `libudev-dev`: For enumerating and introspecting local devices. Not useful in a container context. Note: For use-cases like compiling Python/Ruby/PHP binaries for upload to S3, any required headers (such as `libreadline-dev`) that aren't also needed for *application* dependencies should instead be installed in the image being used to compile those binaries, rather than being included in the build image here. GUS-W-15159536.
Since it was present in Heroku-22 via a transitive dependency, but no longer pulled in on Heroku-24.
edmorley
force-pushed
the
edmorley/cleanup-build-libs
branch
from
May 5, 2024 22:10
f657abc
to
2ce90e6
Compare
edmorley
added a commit
to heroku/docker-heroku-ruby-builder
that referenced
this pull request
May 9, 2024
Since it's no longer in the Heroku-24 build image after: heroku/base-images#296 (The headers are only needed when the Ruby runtimes are being compiled. The run image still has the necessary runtime library counterparts.) See also: heroku/heroku-buildpack-python#1580
edmorley
added a commit
to heroku/docker-heroku-ruby-builder
that referenced
this pull request
May 9, 2024
Since it's no longer in the Heroku-24 build image after: heroku/base-images#296 (The headers are only needed when the Ruby runtimes are being compiled. The run image still has the necessary runtime library counterparts.) See also: heroku/heroku-buildpack-python#1580
edmorley
added a commit
to heroku/heroku-buildpack-python
that referenced
this pull request
May 9, 2024
…1580) Required so that the Python `readline` and `curses` stdlib modules will be built, now that `libreadline-dev` is intentionally no longer in the Heroku-24 build image: heroku/base-images#296 (These headers are only needed when the Python runtimes are being compiled. The run image still has the necessary runtime library counterparts.) GUS-W-15159536.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A number of development packages currently being installed in the build image are actually for use-case that either do not make sense in a container context, or are not used during buildpack execution (ie: aren't needed by either the buildpack directly, or when the apps's dependencies are installed/built).
Many of these packages have been simply been copy-pasted from one new stack to the next, even though they are not actually needed in the image.
As such, the following have been removed from the build image:
libacl
(includingcoreutils
), app dependencies won't be typically linking against it, so we don't need the headers for it.libapt-pkg
andlibapt-inst
. Neither our buildpacks nor apps need to link againstlibapt
since they use theapt
CLI instead.libaudit
is used by things like thelogin
command, but otherwise not typically needed for app dependencies to link against.libkmod
lib is not present in the run image, so this wouldn't be usable at runtime anyway.libpopt
, app dependencies won't be typically linking against it, so we don't need the headers for it.libsystemd0
lib is not present in the run image, so this wouldn't be usable at runtime anyway.Note: For use-cases like compiling Python/Ruby/PHP binaries for upload to S3, any required headers (such as
libreadline-dev
) that aren't also needed for application dependencies should instead be installed in the image being used to compile those binaries, rather than being included in the build image here (example).Towards #266.
GUS-W-15159536.