pre-push: find named remote for URL if possible

Some users use tools like JGit, which use URLs instead of named remotes
when invoking the pre-push hook.  This is problematic because we use
remote-tracking branches to optimize our ref walk and when a user is
using a URL, we can't apply that optimization since we don't know which
named remote is being used.

However, if we have a named remote with only one URL, we can apply this
optimization by looking up the remote name and using that as our remote
instead.  We do this only in the pre-push hook, since this is the major
time that we need to walk the history.

Add some unit and integration tests for this and also update one of the
existing integration tests which was relying on the previous behavior by
switching it to use an equivalent URL instead of the actual remote URL.
This commit is contained in:
brian m. carlson 2020-04-17 14:10:54 +00:00
parent 574855d5a8
commit 92e7a27137
No known key found for this signature in database
GPG Key ID: 2D0C9BC12F82B3A1
4 changed files with 164 additions and 6 deletions

@ -47,7 +47,8 @@ func prePushCommand(cmd *cobra.Command, args []string) {
requireGitVersion()
// Remote is first arg
if err := cfg.SetValidPushRemote(args[0]); err != nil {
remote, _ := git.MapRemoteURL(args[0], true)
if err := cfg.SetValidPushRemote(remote); err != nil {
Exit("Invalid remote name %q: %s", args[0], err)
}

@ -353,6 +353,54 @@ func RemoteList() ([]string, error) {
return ret, nil
}
func RemoteURLs(push bool) (map[string][]string, error) {
cmd := gitNoLFS("remote", "-v")
outp, err := cmd.StdoutPipe()
if err != nil {
return nil, fmt.Errorf("failed to call git remote -v: %v", err)
}
cmd.Start()
defer cmd.Wait()
scanner := bufio.NewScanner(outp)
text := "(fetch)"
if push {
text = "(push)"
}
ret := make(map[string][]string)
for scanner.Scan() {
// [remote, urlpair-text]
pair := strings.Split(strings.TrimSpace(scanner.Text()), "\t")
if len(pair) != 2 {
continue
}
// [url, "(fetch)" | "(push)"]
urlpair := strings.Split(pair[1], " ")
if len(urlpair) != 2 || urlpair[1] != text {
continue
}
ret[pair[0]] = append(ret[pair[0]], urlpair[0])
}
return ret, nil
}
func MapRemoteURL(url string, push bool) (string, bool) {
urls, err := RemoteURLs(push)
if err != nil {
return url, false
}
for name, remotes := range urls {
if len(remotes) == 1 && url == remotes[0] {
return name, true
}
}
return url, false
}
// Refs returns all of the local and remote branches and tags for the current
// repository. Other refs (HEAD, refs/stash, git notes) are ignored.
func LocalRefs() ([]*Ref, error) {

@ -702,3 +702,90 @@ func TestRefTypeKnownPrefixes(t *testing.T) {
assert.Equal(t, expected.Ok, ok)
}
}
func TestRemoteURLs(t *testing.T) {
repo := test.NewRepo(t)
repo.Pushd()
defer func() {
repo.Popd()
repo.Cleanup()
}()
cfg := repo.GitConfig()
cfg.SetLocal("remote.foo.url", "https://github.com/git-lfs/git-lfs.git")
cfg.SetLocal("remote.bar.url", "https://github.com/git-lfs/wildmatch.git")
cfg.SetLocal("remote.bar.pushurl", "https://github.com/git-lfs/pktline.git")
expected := make(map[string][]string)
expected["foo"] = []string{"https://github.com/git-lfs/git-lfs.git"}
expected["bar"] = []string{"https://github.com/git-lfs/wildmatch.git"}
actual, err := RemoteURLs(false)
assert.Nil(t, err)
assert.Equal(t, expected, actual)
expected["bar"] = []string{"https://github.com/git-lfs/pktline.git"}
actual, err = RemoteURLs(true)
assert.Nil(t, err)
assert.Equal(t, expected, actual)
}
func TestMapRemoteURL(t *testing.T) {
repo := test.NewRepo(t)
repo.Pushd()
defer func() {
repo.Popd()
repo.Cleanup()
}()
cfg := repo.GitConfig()
cfg.SetLocal("remote.foo.url", "https://github.com/git-lfs/git-lfs.git")
cfg.SetLocal("remote.bar.url", "https://github.com/git-lfs/wildmatch.git")
cfg.SetLocal("remote.bar.pushurl", "https://github.com/git-lfs/pktline.git")
tests := []struct {
url string
push bool
match bool
val string
}{
{
"https://github.com/git-lfs/git-lfs.git",
false,
true,
"foo",
},
{
"https://github.com/git-lfs/git-lfs.git",
true,
true,
"foo",
},
{
"https://github.com/git-lfs/wildmatch.git",
false,
true,
"bar",
},
{
"https://github.com/git-lfs/pktline.git",
true,
true,
"bar",
},
{
"https://github.com/git-lfs/pktline.git",
false,
false,
"https://github.com/git-lfs/pktline.git",
},
{
"https://github.com/git/git.git",
true,
false,
"https://github.com/git/git.git",
},
}
for _, test := range tests {
val, ok := MapRemoteURL(test.url, test.push)
assert.Equal(t, ok, test.match)
assert.Equal(t, val, test.val)
}
}

@ -1181,6 +1181,28 @@ begin_test "pre-push with pushDefault and explicit remote"
)
end_test
begin_test "pre-push uses optimization if remote URL matches"
(
set -e
reponame="pre-push-remote-url-optimization"
setup_remote_repo "$reponame"
clone_repo "$reponame" "$reponame"
endpoint=$(git config remote.origin.url)
contents_oid=$(calc_oid 'hi\n')
git config "lfs.$endpoint.locksverify" false
git lfs track "*.dat"
echo "hi" > a.dat
git add .gitattributes a.dat
git commit -m "add a.dat"
refute_server_object "$reponame" $contents_oid "refs/heads/master"
GIT_TRACE=1 GIT_TRANSFER_TRACE=1 git push "$endpoint" master 2>&1 | tee push.log
grep 'rev-list.*--not --remotes=origin' push.log
)
end_test
begin_test "pre-push does not traverse Git objects server has"
(
set -e
@ -1198,10 +1220,10 @@ begin_test "pre-push does not traverse Git objects server has"
refute_server_object "$reponame" $contents_oid "refs/heads/master"
# We use a URL instead of a named remote so that we can't make use of the
# optimization that ignores objects we already have in remote tracking
# branches.
GIT_TRACE=1 GIT_TRANSFER_TRACE=1 git push "$endpoint" master 2>&1 | tee push.log
# We use a different URL instead of a named remote or the remote URL so that
# we can't make use of the optimization that ignores objects we already have
# in remote tracking branches.
GIT_TRACE=1 GIT_TRANSFER_TRACE=1 git push "$endpoint.git" master 2>&1 | tee push.log
assert_server_object "$reponame" $contents_oid "refs/heads/master"
@ -1212,7 +1234,7 @@ begin_test "pre-push does not traverse Git objects server has"
refute_server_object "$reponame" $contents2_oid "refs/heads/master"
GIT_TRACE=1 GIT_TRANSFER_TRACE=1 git push "$endpoint" master 2>&1 | tee push.log
GIT_TRACE=1 GIT_TRANSFER_TRACE=1 git push "$endpoint.git" master 2>&1 | tee push.log
assert_server_object "$reponame" $contents2_oid "refs/heads/master"