Repository navigation
gitcache: decide which git fetches can be cached, and their key - #163
Merged
Merged
Conversation
A git checkout cache needs to tell a fetch whose answer depends on the request alone from one whose answer does not. Parse accepts a protocol v2 fetch only when it understands every line, the client has no objects, and the wants are exact commits. Anything else goes to GitHub, so a line from a newer git costs a miss rather than a wrong answer, and want-ref, whose answer moves, is never cached. Nothing uses this yet. The instance's proxy will store through cache-server's existing API, which already scopes entries by repository and ref, so this adds no endpoint. Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
Ref-dependent filters and include-tag can produce different object sets under the same cache key, while duplicate commands and malformed flag arguments are incorrectly accepted.
4 open issues
What changed in this PR
Introduces the request-classification and cache-key foundation for EC2 git checkout caching. The new package is not yet connected to a proxy or cache server.
Changes:
- Parses protocol v2 fetch requests and rejects unsupported request forms.
- Generates repository-scoped keys from fetch options and normalizes repository paths.
- Adds parsing, key-isolation, repository-path, and fuzz tests.
| File | Description |
|---|---|
internal/gitcache/request.go |
Implements fetch parsing, cache keys, and repository normalization. |
internal/gitcache/request_test.go |
Tests accepted and rejected requests, key behavior, and parser invariants. |
AGENTS.md |
Adds the gitcache package to the repository directory guide. |
Review effort: Balanced
sparse:oid can name its pattern through a ref, such as HEAD:patterns, and combine: can hold one, so the same request can mean different objects after that ref moves. Allow blob:none, blob:limit and tree alone. Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
Its answer holds whatever annotated tags point at the wants, which changes without the request changing. Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
upload-pack rejects it, so a hit must not answer it. Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
Cutting at the space accepted `done extra` as done, which upload-pack rejects. Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
Signed-off-by: Yi-Ru Lin <deduce@gmail.com>
Contributor
Author
|
babysit-pr · head |
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
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.
First part of the git checkout cache for EC2 tiers. Nothing calls this yet.
Parsereads the body of agit-upload-packPOST and accepts it only when the response depends on the request alone:command=fetchhaveorshallow, so the client has no objectsdonepresent, so negotiation is finishedwantlines holding complete object IDsEverything else returns
ErrNotCacheableand goes to GitHub. It is an allow list, so a line from a newer git costs a miss, not a wrong answer. Three things are refused because their answer moves with the repository:want-ref,include-tag, andsparse:oidfilters, alone or insidecombine:.Keyhashes the repository and what changes the objects: wants, depth, filter,thin-pack,ofs-deltaand the object format. It leaves outagent,session-idandno-progress, so git versions andactions/checkout's arguments share an entry. Real requests from git 2.56 are the test's fixtures;git fetch --depth=1andactions/checkout's arguments send the same lines.Why no cache-server change
An entry there is already repository, ref, key and version, with scope checks, eviction, size limits and S3. The instance's proxy will call the same API as
actions/cache, withgit/<hash>as the key, so isolation is inherited and the security surface does not grow.Verified
FuzzParsefor 15 s.scripts/mutate.shagainst 23 guards. The first pass left three surviving, a missing two-command case, unordered wants and a field-separation test that proved nothing. All three now fail a test.Replaying through real git was done in a proof of concept, not here. That belongs with the proxy.