Skip to content

Adding usage of git_create_remote_detached #755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding usage of git_create_remote_detached #755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ const (

// RemoteCreateOptions contains options for creating a remote
type RemoteCreateOptions struct {
Name string
FetchSpec string
Flags RemoteCreateOptionsFlag
Name string
FetchSpec string
Flags RemoteCreateOptionsFlag
}

type TransferProgress struct {
Expand Down Expand Up @@ -173,6 +173,37 @@ type Remote struct {
repo *Repository
}

type RemoteDetached struct {
r Remote
}

// NewRemoteDetached returns a detached remote that has no
// reference to a repository. Used to emulate `git ls-remote <repo-url>`
func NewRemoteDetached(remoteUri string) (*RemoteDetached, error) {
var ptr *C.git_remote

runtime.LockOSThread()
defer runtime.UnlockOSThread()

ret := C.git_remote_create_detached(&ptr, C.CString(remoteUri))
if ret != 0 {
return nil, MakeGitError(ret)
}

return &RemoteDetached{r: Remote{ptr: ptr}}, nil
Copy link
Contributor

@lhchavez lhchavez Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

would it be possible for this to return the *Remote directly? That way the RemoteDetached and the LsDetached shouldn't be needed, since callers can interact with the remote using the normal API, which will avoid having to implement more APIs on RemoteDetached that would only forward arguments to the underlying Remote. It's totally fine if the repo property is nil: it's just there to avoid the GC from deleting the Repository when the Remote is still around, but here it will be nil.

also, the finalizer for the Remote object must be set prior to returning to avoid memory leaks:

git2go/remote.go

Lines 586 to 587 in 75708ee

runtime.SetFinalizer(remote, (*Remote).Free)
return remote, nil

}

// LsDetached returns a listing of []RemoteHead given an optional reference filter,
// emulating `git ls-remote ...`
func (rd *RemoteDetached) LsDetached(fetchOpts FetchOptions, filterRefs ...string) ([]RemoteHead, error) {
err := rd.r.ConnectFetch(&fetchOpts.RemoteCallbacks, &fetchOpts.ProxyOptions, fetchOpts.Headers)
if err != nil {
return nil, err
}

return rd.r.Ls(filterRefs...)
}

type CertificateKind uint

const (
Expand Down
51 changes: 45 additions & 6 deletions remote_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"golang.org/x/crypto/ssh"
)

const TestGitRepoUri = "https://github.com/libgit2/TestGitRepository"

func TestListRemotes(t *testing.T) {
t.Parallel()
repo := createTestRepo(t)
Expand Down Expand Up @@ -51,7 +53,7 @@ func TestCertificateCheck(t *testing.T) {
repo := createTestRepo(t)
defer cleanupTestRepo(t, repo)

remote, err := repo.Remotes.Create("origin", "https://github.com/libgit2/TestGitRepository")
remote, err := repo.Remotes.Create("origin", TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

Expand All @@ -72,7 +74,7 @@ func TestRemoteConnect(t *testing.T) {
repo := createTestRepo(t)
defer cleanupTestRepo(t, repo)

remote, err := repo.Remotes.Create("origin", "https://github.com/libgit2/TestGitRepository")
remote, err := repo.Remotes.Create("origin", TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

Expand All @@ -95,7 +97,7 @@ func TestRemoteConnectOption(t *testing.T) {
option.Name = "origin"
option.Flags = RemoteCreateSkipInsteadof

remote, err := repo.Remotes.CreateWithOptions("https://github.com/libgit2/TestGitRepository", option)
remote, err := repo.Remotes.CreateWithOptions(TestGitRepoUri, option)
checkFatal(t, err)

err = remote.ConnectFetch(nil, nil, nil)
Expand All @@ -107,7 +109,7 @@ func TestRemoteLs(t *testing.T) {
repo := createTestRepo(t)
defer cleanupTestRepo(t, repo)

remote, err := repo.Remotes.Create("origin", "https://github.com/libgit2/TestGitRepository")
remote, err := repo.Remotes.Create("origin", TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

Expand All @@ -127,7 +129,7 @@ func TestRemoteLsFiltering(t *testing.T) {
repo := createTestRepo(t)
defer cleanupTestRepo(t, repo)

remote, err := repo.Remotes.Create("origin", "https://github.com/libgit2/TestGitRepository")
remote, err := repo.Remotes.Create("origin", TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

Expand Down Expand Up @@ -162,7 +164,7 @@ func TestRemotePruneRefs(t *testing.T) {
err = config.SetBool("remote.origin.prune", true)
checkFatal(t, err)

remote, err := repo.Remotes.Create("origin", "https://github.com/libgit2/TestGitRepository")
remote, err := repo.Remotes.Create("origin", TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

Expand Down Expand Up @@ -493,3 +495,40 @@ func TestRemoteSSH(t *testing.T) {
t.Error("Expected remote heads")
}
}

func TestNewRemoteDetached(t *testing.T) {
_, err := NewRemoteDetached(TestGitRepoUri)
if err != nil {
t.Fatalf("error %v", err)
}
}

func TestRemoteDetached_LsDetached(t *testing.T) {
r, _ := NewRemoteDetached(TestGitRepoUri)
Copy link
Contributor

@lhchavez lhchavez Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
r, _ := NewRemoteDetached(TestGitRepoUri)
remote, err := NewRemoteDetached(TestGitRepoUri)
checkFatal(t, err)
defer remote.Free()

for consistency with other tests that create remotes (and to avoid memory leaks)


t.Run("NoInput", func(t *testing.T) {
rh, err := r.LsDetached(FetchOptions{})
if err != nil {
t.Fatalf("error %v", err)
}

if len(rh) == 0 {
t.Fatalf("Not listing all references")
}
})

t.Run("Filters", func(t *testing.T) {
rh, err := r.LsDetached(FetchOptions{}, "HEAD")
if err != nil {
t.Fatalf("error %v", err)
}

if len(rh) == 0 {
t.Fatalf("Not listing all references")
}

if rh[0].Name != "HEAD" {
t.Fatalf("Not listing all references")
}
})
}