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

docker: enable more caching #10974

Merged
merged 7 commits into from
Oct 10, 2024
Merged

docker: enable more caching #10974

merged 7 commits into from
Oct 10, 2024

Conversation

pjonsson
Copy link
Contributor

@pjonsson pjonsson commented Oct 9, 2024

This changes a lot of things all over the place, but the big picture is:

  • Release builds pass --no-cache to Docker so they don't accidentally pick up something from the Docker cache. For local experimentation, that can be overridden by passing --docker-cache to build.sh.
  • The deb packages+lists are cached in Docker for non-release builds. This avoids fetching the same thing from the network repeatedly. The cache for apt can only be used by one build at the time so that is mounted with sharing=locked. To avoid blocking concurrent docker image builds, apt-get calls were put in a separate RUN statement from a long-running compilation.
  • Separation of rsync and ccache behavior, which lets the user disable the gdal rsync daemon but still benefit from ccache and the Docker cache.

It's always tempting to change everything at once, but I have tried to restrain myself so this PR would still be reviewable. Each commit does one thing, and viewing the commit without showing whitespace changes should give a reasonably sized change.

I have built all images locally with both --release and without it.

docker/util.sh Outdated Show resolved Hide resolved
@rouault
Copy link
Member

rouault commented Oct 9, 2024

shellcheck is complaining:

shellcheck -e SC2086,SC2046,SC2164,SC2054,SC2129 $(find . -name '*.sh' -a -not -name ltmain.sh -a -not -wholename "./autotest/*" -a -not -wholename "./.github/*")

In ./docker/util.sh line 216:
    $DOCKER_CACHE_PARAM \
    ^-----------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

For more information:
  https://www.shellcheck.net/wiki/SC2206 -- Quote to prevent word splitting/g...

Add --docker-cache/--no-docker-cache parameters
to the script, and default to --no-docker-cache
when building releases.

Having the on/off option defaults to
the safe choice, but when explicitly passed,
it's possible to do local experimentation
with building the release image and
benefitting from the cache.
Use a colon like the other lines.
The BUILD_ARGS array is mostly the same
for both branches, so hoist the definition
and append the unique parts inside each
branch.
Enable building without the rsync daemon.

This is preparation for being able
to use ccache and Docker caching
without syncing with the home directory.
As a bonus, no rsync daemon means
the build can use the default network
instead of host, so more encapsulation.
Permit building images with ccache
and caching inside Docker, but
without syncing the cache to the home
directory.
Hoist the installation of ccache and
rsync into one of the early blocks
in all images. This ensures that
the ccache binary is available
for everything built.
Use the docker cache for package lists
and deb files. This avoids updating
the same lists multiple times in
the same build, and enables consecutive
builds to install packages without
downloading them from internet.
@rouault rouault merged commit a7db122 into OSGeo:master Oct 10, 2024
15 checks passed
@pjonsson pjonsson deleted the docker-more-cache branch October 10, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants