git source: add AttrGitChecksum #5975
Conversation
github-actions
bot
added
area/llb
area/testing
area/client
area/solver
area/source
labels
source/git/source.go
Outdated
| refCommitFullHash = gs.src.Ref | ||
| } | ||
| if refCommitFullHash != "" { | ||
| cacheKey := gs.shaToCacheKey(refCommitFullHash, "") |
There was a problem hiding this comment.
if the checksum is set together with the ref then the ref should be passed here as well to shaToCacheKey. If checking out different branches with the same checksum they can be different after checkout with keep-git-dir.
There is still bit of a difference between the cache key generated with checksum compared to the one generated without it because in here we only have access to ref passed by user while in other code-path we use usedRef (if we have full ref like refs/heads, refs/tags then they should be the same though). Iiuc we could use the keep-git-dir option.
If keep-git and text ref then resolve text ref as done without the checksum. If checksums don't match can error right away. Cache key would be gs.shaToCacheKey(sha, usedRef).
If no keep-git-dir then we could just use the sha(either from ref or from checksum) as a cache key without remote resolve. shaToCacheKey already ignores ref if no keep-git-dir.
There was a problem hiding this comment.
shaToCacheKey
done
usedRef
This might be beyond the scope of this PR?
There was a problem hiding this comment.
This might be beyond the scope of this PR?
Changing it later would break cache though.
There was a problem hiding this comment.
Added testcase for this 427305b
Atm cache keys would be different with 2799cae6ffb0390691ecf8f6ca9c6371c4a6dfe3.git#refs/tags/v0.0.2 for first build and 2799cae6ffb0390691ecf8f6ca9c6371c4a6dfe3.git#v0.0.2?checksum=2799cae6ffb0390691ecf8f6ca9c6371c4a6dfe3 for the second one.
There was a problem hiding this comment.
Thanks, cherry-picked your commit
There was a problem hiding this comment.
Dropped ?checksum=... from shaToCacheKey, as originally suggested in #5903 (comment)
The test should pass now.
Not integrated to util/giturl, as PR 5974 is not merged yet. Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
| } | ||
|
|
||
| var refCommitFullHash, ref2 string | ||
| if gitutil.IsCommitSHA(gs.src.Checksum) && !gs.src.KeepGitDir { |
There was a problem hiding this comment.
Can be follow-up: I think if gs.src.Ref is a full ref like refs/tags/..., refs/heads/... pull/NR/head etc. then we can do early return in here even if gs.src.KeepGitDir is true.
Split from:
Not integrated to util/giturl, as the following PR is not merged yet.