wip/core(git): opportunistic git fetch#11833
Draft
grouville wants to merge 7 commits intodagger:mainfrom
Draft
Conversation
281ccca to
539dc50
Compare
This lazifies the git API and makes resolution (`_resolve`) the only place where we touch remote state. Pure, and auth/remote lookup only happens when we resolve them. Resolve paths (`tree`, `branches`, `tags`) run under `CachePerClient`. So, in one-shot CLI usage, auth is effectively checked once per command (same as `main`), while long-lived clients reuse the same resolved value within. Cache identity is based on semantics: 1. *repo identity* includes access method (protocol + auth inputs) and remote snapshot (`ls-remote`), so auth contexts do not collide and pushes invalidate cache. 2. *ref identity* is based on resolved ref target (name + commit) on top of repo identity. 3. *tree identity* is based on checkout shape (`commit/depth/discardGitDir`) after resolve, so equivalent content trees can be shared without skipping auth checks. Safeguard: any path that resolves git state (and therefore evaluates auth or remote state) MUST go through a `CachePerClient` resolve boundary, now Signed-off-by: Guillaume de Rouville <[email protected]>
After dagger#11828, lazy git resolution routes SSH auth through host._sshAuthSocket. When SSH_AUTH_SOCK is missing, that surfaced a host-level error string and regressed cross-session auth-leak expectations. Keep the host cause typed and map it back to the existing git-facing auth error in repo.__resolve so behavior stays stable without changing cache identity semantics. Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
539dc50 to
278a6cb
Compare
Signed-off-by: Guillaume de Rouville <[email protected]>
79db94e to
82cc728
Compare
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.
Follow-up of git lazification, on top of #11560
For the hot path:
dag.Git().Head().tree(), instead of always:We should be able to only
fetchdirectly to avoid this unnecessary step. This has implications on the function caching, but discussion is open on thatThis is an ugly solution still, but the aim is to assess the performance across the entire test suite in order to assess whether it's a worthwhile change