Skip to content

/{go,integration-tests}: handle scp style git remote urls better#10640

Merged
coffeegoddd merged 4 commits intomainfrom
db/fix-gr
Mar 5, 2026
Merged

/{go,integration-tests}: handle scp style git remote urls better#10640
coffeegoddd merged 4 commits intomainfrom
db/fix-gr

Conversation

@coffeegoddd
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves handling of SCP-style Git remotes ([user@]host:path.git) so relative paths don’t get misinterpreted as absolute paths when Dolt normalizes URLs and later invokes git.

Changes:

  • Normalize SCP-style relative remotes to git+ssh://..././path.git to preserve “relative path” semantics.
  • Reconstruct SCP-style host:path when passing such remotes to git (instead of ssh://host/path, which git treats as absolute).
  • Add/update Bats + Go tests covering the regression and new normalization behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
integration-tests/bats/remotes-git.bats Adds regression test and a fake git wrapper to record the URL Dolt passes to git remote add.
integration-tests/bats/remote-cmd.bats Updates expected normalized git+ssh output to include /./ marker.
go/libraries/doltcore/env/git_remote_url.go Changes normalization of SCP-style remotes to include /./ for relative paths.
go/libraries/doltcore/env/git_remote_url_test.go Updates / adds unit tests for the revised SCP-style normalization rules.
go/libraries/doltcore/dbfactory/git_remote.go Converts ssh URLs with /./ back into SCP-style when invoking git for remote setup/validation.
go/libraries/doltcore/dbfactory/git_remote_test.go Adds unit tests for the ssh→SCP reconstruction helper.
Comments suppressed due to low confidence (1)

go/libraries/doltcore/env/git_remote_url.go:90

  • This change updates SCP-style normalization to include a /./ marker (e.g. git@host:path.gitgit+ssh://..././path.git). There is at least one existing Go unit test that still asserts the old value (e.g. go/cmd/dolt/commands/remote_test.go:159 expects no /./), which will fail once this behavior changes. Please update any remaining expectations to the new normalized form (or loosen them to accept both, if backward compatibility is required).
	// scp-like ssh: [user@]host:path/repo.git (no scheme, no ://)
	if isScpLikeGitRemote(urlArg) {
		host, p := splitScpLike(urlArg)
		var pathPart string
		if strings.HasPrefix(p, "/") {
			// Absolute path: git@host:/abs/repo.git → /abs/repo.git
			pathPart = p
		} else {
			// Relative path: git@host:path.git → /./path.git
			pathPart = "/./" + p
		}
		ssh := "git+ssh://" + host + pathPart
		u, err := url.Parse(ssh)
		if err != nil {
			return "", false, err
		}
		return u.String(), true, nil

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coffeegoddd
Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
3e94572 ok 5937471
version total_tests
3e94572 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
069fce8 ok 5937471
version total_tests
069fce8 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor Author

Fixes #10564

@coffeegoddd coffeegoddd requested a review from macneale4 March 5, 2026 20:07
@coffeegoddd
Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

read_tests from_latency to_latency percent_change
covering_index_scan 0.55 0.56 1.82
groupby_scan 9.91 10.09 1.82
index_join 1.82 1.86 2.2
index_join_scan 1.34 1.37 2.24
index_scan 21.89 21.89 0.0
oltp_point_select 0.27 0.27 0.0
oltp_read_only 5.28 5.28 0.0
select_random_points 0.54 0.54 0.0
select_random_ranges 0.55 0.56 1.82
table_scan 22.28 22.28 0.0
types_table_scan 66.84 66.84 0.0
write_tests from_latency to_latency percent_change
oltp_delete_insert 6.32 6.43 1.74
oltp_insert 3.13 3.13 0.0
oltp_read_write 11.45 11.45 0.0
oltp_update_index 3.19 3.19 0.0
oltp_update_non_index 3.13 3.13 0.0
oltp_write_only 6.09 6.09 0.0
types_delete_insert 6.79 6.91 1.77

@coffeegoddd
Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
2481b9f ok 5937471
version total_tests
2481b9f 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coffeegoddd coffeegoddd merged commit 0e4e7d5 into main Mar 5, 2026
26 of 27 checks passed
@coffeegoddd coffeegoddd deleted the db/fix-gr branch March 5, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants