-
Notifications
You must be signed in to change notification settings - Fork 19
Refactor Dockerfiles for improved caching performance #997
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
Draft
Ishirui
wants to merge
20
commits into
main
Choose a base branch
from
pierrelouis.veyrenc/ACIX-1056-improve-dockerfile-layering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Refactor Dockerfiles for improved caching performance #997
Ishirui
wants to merge
20
commits into
main
from
pierrelouis.veyrenc/ACIX-1056-improve-dockerfile-layering
Conversation
This file contains hidden or 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
7b7bba4 to
5171bb8
Compare
5da8c5f to
15c1b8a
Compare
This has a pretty big impact on the cache invalidation logic. When we mount the entire repo, like was done before, the cached layer gets invalidated when there is _any_ change to _any_ file in the repo, no matter if it actually impacts the layer or not. By only mounting the individual script file, we make sure that only changes to the script file affect the build.
…curl` or `wget` This is more integrated with the Docker build, and in particular allows the Docker builder to parallelize downloads of everything it needs to include, and start these downloads at the very beginning of the build process. This also uses a Docker-native way of verifying checksums, and is in general a better practice - although it results in a couple more layers (will not be important once we switch to a multistage build)
Standardizing on one format for arch definition, and only specifying aliases when useful will make it a lot easier to read and implement. Using the builtin `BUILDARCH` also avoids us needing to pass in an extra build arg for the build platform.
This allows for better layer caching and smaller final image size. This also accelerates the build process as all stages are built in parallel.
This allows for improved caching performance, as changing a build arg now only invalidates the stage that uses it, instead of the entire base stage.
For some reason crosstool-ng does not build properly without texinfo being built first. Seems to be some issue with the libc install that gets resolved by building texinfo first.
feat(linux): Simplify up go, python and bazel install scripts for multiarch We move a lot of the logic from the shell script for these tools directly into the dockerfile/bakefile: - Having the versions and checksums in the bakefile allows us to have a single source of truth - Having them in the bakefile means we can more easily pass different values based on arch, instead of relying on shell-based ways of detecting the build architecture - Using `ADD` directives instead of manual calls to `curl` is more idiomatic and also allows for better performance (works better with caching + parallel downloads) - Add a couple more args and split things up (e.g. dda install script) to improve readability and uniformity of all the stages refactor(linux): Use all tool-specific ARCH definitions This makes it clearer which argument is used where, and avoids needing globally used `ARGs` (better for caching) refactor(linux): Remove unused `build.env` files fix(linux): Propery pass variables to LABEL instructions
These new scripts are now for the most part incompatible with the legacy images, so might as well move them to the `linux/` directory
+ Restore old setup scripts for legacy images
7492e0e to
cf0bf17
Compare
89bd3e9 to
3579c10
Compare
…CIX-1056-improve-dockerfile-layering
3579c10 to
f26524b
Compare
…CIX-1056-improve-dockerfile-layering
9d13e64 to
07cb5b7
Compare
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.
What does this PR do?
Motivation
Possible Drawbacks / Trade-offs
Additional Notes