Add git+* dbfactory remotes with required --git-cache-dir, --ref support, and integration tests#10483
Add git+* dbfactory remotes with required --git-cache-dir, --ref support, and integration tests#10483coffeegoddd merged 20 commits intomainfrom
git+* dbfactory remotes with required --git-cache-dir, --ref support, and integration tests#10483Conversation
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
There was a problem hiding this comment.
Pull request overview
Adds Git-backed Dolt remotes via dbfactory using git+* URL schemes, wiring support through CLI + SQL procedures and validating behavior with new end-to-end and unit tests.
Changes:
- Introduce a
GitRemoteFactory(git+file/http/https/ssh) with local bare-repo caching and configurablegit_ref. - Normalize common git remote strings (scp-style, schemeless host/path,
.gitURLs, local paths) into canonicalgit+*URLs. - Add
--git-cache-dir/--refplumbing across CLI commands + SQL procedures, plus BATS + Go tests for push/clone/pull and empty-remote bootstrap.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| integration-tests/bats/sql-remotes-git.bats | SQL procedure integration tests for --ref behavior on git remotes. |
| integration-tests/bats/remotes-git.bats | End-to-end git remote smoke tests (push/clone/pull) + bootstrap + default cache-dir behavior. |
| go/libraries/doltcore/sqle/dprocedures/dolt_remote.go | Adds git remote parameter support to dolt_remote('add', ...). |
| go/libraries/doltcore/sqle/dprocedures/dolt_clone.go | Adds git remote parameter support + URL normalization fallback for SQL dolt_clone. |
| go/libraries/doltcore/sqle/dprocedures/dolt_backup.go | Adds git remote parameter handling to backup URL parsing. |
| go/libraries/doltcore/env/remotes.go | Plumbs remote name into git-remote params; normalizes git URLs inside GetAbsRemoteUrl. |
| go/libraries/doltcore/env/git_remote_url.go | New URL normalization helper for recognizing/rewriting git remote strings. |
| go/libraries/doltcore/env/git_remote_url_test.go | Unit tests for NormalizeGitRemoteUrl. |
| go/libraries/doltcore/dbfactory/git_remote.go | New GitRemoteFactory implementation (cache repo + git remote config + NBS Git store). |
| go/libraries/doltcore/dbfactory/git_remote_test.go | Go tests validating cache dir usage and multi-client roundtrip. |
| go/libraries/doltcore/dbfactory/factory.go | Registers git+* schemes in the dbfactory map. |
| go/cmd/dolt/commands/remote.go | Adds --git-cache-dir / --ref parsing and scheme gating for CLI dolt remote. |
| go/cmd/dolt/commands/remote_test.go | Tests for git remote flag parsing. |
| go/cmd/dolt/commands/read_tables.go | Adds git flags to arg parser and URL normalization fallback for read-tables. |
| go/cmd/dolt/commands/read_tables_test.go | Verifies read-tables parser accepts git flags. |
| go/cmd/dolt/commands/clone.go | Adds URL normalization fallback during clone arg parsing. |
| go/cmd/dolt/commands/clone_test.go | Tests clone directory inference behavior. |
| go/cmd/dolt/cli/arg_parser_helpers.go | Adds git flags to clone/remote/backup arg parsers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ref != "" { | ||
| if !isGitRemote { | ||
| return nil, errhand.BuildDError("error: --ref is only supported for git remotes").Build() | ||
| } | ||
| remoteParms[dbfactory.GitRefParam] = ref | ||
| } |
There was a problem hiding this comment.
The --ref flag handling treats an empty value as "not provided" (no error). In other interfaces (e.g. dolt remote add), an empty --ref is rejected. Consider making this consistent by erroring if --ref is present but empty, instead of silently ignoring it.
| if ref != "" { | |
| if !isGitRemote { | |
| return nil, errhand.BuildDError("error: --ref is only supported for git remotes").Build() | |
| } | |
| remoteParms[dbfactory.GitRefParam] = ref | |
| } | |
| if ref == "" { | |
| return nil, errhand.BuildDError("error: --ref cannot be empty").Build() | |
| } | |
| if !isGitRemote { | |
| return nil, errhand.BuildDError("error: --ref is only supported for git remotes").Build() | |
| } | |
| remoteParms[dbfactory.GitRefParam] = ref |
| } | ||
|
|
||
| func looksLikeLocalPath(s string) bool { | ||
| return strings.HasPrefix(s, "/") || strings.HasPrefix(s, "./") || strings.HasPrefix(s, "../") |
There was a problem hiding this comment.
looksLikeLocalPath() only checks for '/', './', and '../' prefixes. On Windows, absolute paths like "C:\repos\remote.git" (or UNC paths) won't be treated as local paths, so NormalizeGitRemoteUrl will misclassify them as schemeless host/path and return a git+https URL. Consider using filepath.IsAbs / filepath.VolumeName (and UNC checks) to detect Windows absolute paths reliably.
| return strings.HasPrefix(s, "/") || strings.HasPrefix(s, "./") || strings.HasPrefix(s, "../") | |
| if s == "" { | |
| return false | |
| } | |
| // Use filepath.IsAbs so that absolute paths are detected according to the | |
| // current OS semantics (e.g. "C:\repo" on Windows, "/repo" on POSIX). | |
| if filepath.IsAbs(s) { | |
| return true | |
| } | |
| // Preserve existing behavior for common relative path prefixes. | |
| if strings.HasPrefix(s, "./") || strings.HasPrefix(s, "../") { | |
| return true | |
| } | |
| // Be robust for UNC-like paths even when running on non-Windows systems. | |
| // UNC paths typically start with "\\" or "//". | |
| if strings.HasPrefix(s, `\\`) || strings.HasPrefix(s, "//") { | |
| return true | |
| } | |
| return false |
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
reltuk
left a comment
There was a problem hiding this comment.
LGTM. A couple comments on bats test.
I reached out on Discord regarding git remote name and an inconsistency in behavior between multi Dolt repositories using the same remote name and same cache dir vs. a single Dolt repository using multiple remotes with different names and the same remote url. We should sync up on approach there as well.
I'm working on landing updates to lambdabats so it has git available.
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
This PR is being tested for performance. Please allow ~12 mins for this to complete. If this PR does not result in a performance regression, the |
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
Implements git-backed Dolt remotes via dbfactory (git+file/http/https/ssh), requiring --git-cache-dir and supporting --ref, with end-to-end BATS + Go multi-client coverage for push/clone/pull and empty-remote bootstrap.