Bazel: allow LFS rules to use cached downloads without internet #16522
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
If the cache is prefilled, LFS rules were still trying to query LFS urls. Now the strategy is to first try to fetch the files from the repository cache (which is possible by providing an empty url list and `allow_fail` to `repository_ctx.download`), and only run the LFS protocol if that fails. Technically this is possible by enhancing `git_lfs_probe.py` with a `--hash-only` flag. This is also an optimization where no uneeded access is done (including the slightly slow SSH call) if the repository cache is warm.
criemen
approved these changes
May 21, 2024
| if extract: | ||
| for src in srcs: | ||
| repository_ctx.report_progress("extracting %s" % src.basename) | ||
| repository_ctx.extract(src.basename, stripPrefix = stripPrefix) |
There was a problem hiding this comment.
Is this now slower on CI (with an empty repository cache) where we'll get a separate download and extract step, or is download_and_extract performance-wise 1. download 2. extract with no overlap anyways?
There was a problem hiding this comment.
Didn't really hard measure it, but it did not seem to make a difference locally.
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.
If the cache is prefilled, LFS rules were still trying to query for LFS urls.
Now the strategy is to first try to fetch the files from the repository cache (which is possible by providing an empty url list and
allow_failtorepository_ctx.download), and only run the LFS protocol if that fails. Technically this is made possible by enhancinggit_lfs_probe.pywith a--hash-onlyflag.This is also an optimization where no uneeded internet access is done (including the slightly slow SSH call) if the repository cache is warm.
Additionally,
-oStrictHostKeyChecking=accept-newis now passed to the SSH call for LFS authentication. This makes a difference in the case the build is executed within a container on an already checked-out repository.