HTTPS Clone fails with remote pointer not found using Go transport

Given:

All tests (even Clone) initialize a Repository before they run a test
The tests that make use of Clone seem to be using a local file path
The limited number of examples available

I am not really sure if this is due to a misuse of the API, or a bug. In any case, the following used to work before not too long ago with git2go/v31 and backing crypto / transport dependencies:

func TestRemoteClone(t *testing.T) {
	t.Parallel()
	path, err := ioutil.TempDir("", "git2go")
	if err != nil {
		t.Fatalf("Failed to init temp dir")
	}
	defer os.RemoveAll(path)
	remote, err := Clone("https://github.com/libgit2/libgit2", path, &CloneOptions{})
	if err != nil {
		t.Fatalf("Clone() = %v, want success", err)
	}
	defer remote.Free()
}

While attempting to update to git2go/v32 however, and trying to depend on less C libraries, this now returns a remote pointer not found error:

=== RUN   TestRemoteClone
=== PAUSE TestRemoteClone
=== CONT  TestRemoteClone
    remote_test.go:94: Clone() = remote pointer not found, want success
--- FAIL: TestRemoteClone (0.00s)

Following the breadcrumbs, it seems to end on https://github.com/libgit2/git2go/blob/main/clone.go#L43 returning -7 (which makes me wonder even more).

I am facing the same issue. I wonder if this has something to do with compiling libgit2 with the openSSL and libSSH libraries.

The language around these is a bit confusing, so I wonder if this is one potential problem

Following the introduction of Go native transports, the configuration to link against these libraries was disabled in this repository: https://github.com/libgit2/git2go/blob/main/script/build-libgit2.sh#L76-L77.

However, even after spending hours trying various configuration options, we have not been able to get a working version of v32, with or without the crypto libs. All eventually failing in some step of the Clone operation.

I am not sure how go native transports work, but let me tinker around and see if I can get it to work. I am still relatively new to this CGO and cross-language compiling. I will keep posting my findings, lets hope something works :P

After a bit of tinkering, I have turned on the HTTPS flag
-DUSE_HTTPS=ON

now when I try to clone, I get the error

# github.com/codexetreme/monorepo-semver-cli/git_bindings.test
/home/codexetreme/sdk/go1.16/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/bin/ld: /home/codexetreme/src/monorepo-semver-cli/third_party/git2go/static-build/install/lib/libgit2.a(openssl.c.o): in function `shutdown_ssl':
/home/codexetreme/src/monorepo-semver-cli/third_party/git2go/vendor/libgit2/src/streams/openssl.c:54: undefined reference to `BIO_meth_free'
/usr/bin/ld: /home/codexetreme/src/monorepo-semver-cli/third_party/git2go/vendor/libgit2/src/streams/openssl.c:59: undefined reference to `SSL_CTX_free'
/usr/bin/ld: /home/codexetreme/src/monorepo-semver-cli/third_party/git2go/static-build/install/lib/libgit2.a(openssl.c.o): in function `bio_destroy':
/home/codexetreme/src/monorepo-semver-cli/third_party/git2go/vendor/libgit2/src/streams/openssl.c:238: undefined reference to `BIO_set_data'
/usr/bin/ld: /home/codexetreme/src/monorepo-semver-cli/third_party/git2go/static-build/install/lib/libgit2.a(openssl.c.o): in function `bio_create':
....

this leads me to believe that simply linking the OpenSSL library is the issue now

UPDATE: I got it to work, (ie, it is not throwing any more SSL errors)
but a few more errors with setting cert checks and verification remain.

Currently the new bug is

go test -v -tags static ./git_bindings
=== RUN   Test_clone
time="2021-10-08T17:26:48+05:30" level=info msg="starting commit: https://github.com/ThreeDotsLabs/wild-workouts-go-ddd-example"
time="2021-10-08T17:26:49+05:30" level=error msg=error error="user cancelled hostkey check"
--- PASS: Test_clone (1.33s)

EDIT: I will update my process of getting this whole thing to work once I iron out the kinks

@hiddeco

@hiddeco using your test, I am able to get it working with recompiling the vendored libgit2 with
CMAKE -DUSE_HTTPS=ON in scripts/build-libgit2.sh

my setup is probably slightly different from yours since I am packaging gogit2 as a submodule, but I believe that is the minimal tweak required.

also, my tests were cached, so I had to use go clean -testcache and re run

yikes, this seems to be caused by the fact that libgit2 duplicates the remote while cloning, so git2go can't match the original Remote object to the one passed into the callback :/ we probably need a different way of matching the remote instead of by-pointer.

But why does it work with the openssl libraries activated?

…

________________________________ From: lhchavez ***@***.***> Sent: Monday, October 11, 2021 5:28:44 PM To: libgit2/git2go ***@***.***> Cc: Yashodhan Ghadge ***@***.***>; Comment ***@***.***> Subject: Re: [libgit2/git2go] HTTPS `Clone` fails with `remote pointer not found` using Go transport (#836) yikes, this seems to be caused by the fact that libgit2 duplicates the remote while cloning, so git2go can't match the original Remote object to the one passed into the callback :/ we probably need a different way of matching the remote instead of by-pointer. — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#836 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACCHNM4CNQ73LKSYNNPUK2LUGLYBZANCNFSM5E6KTZ2A>.

But why does it work with the openssl libraries activated?

that uses a different codepath that doesn't involve the remote callbacks.

Oh, if you have some pointers (pun intended) could you let me know? I would like to work on this if it does turn out to be a bug.

Oh, if you have some pointers (pun intended) could you let me know? I would like to work on this if it does turn out to be a bug.

it is indeed a bug. what should happen is that given that we have no way of recovering the original Remote (due to the underlying pointers being different), if we receive something that is not in our list of remotes, maybe we can create a new Remote object backed by the *git_remote that's passed into the smartTransportCallback. but then we need to handle that with care because we can't just free it: that would break libgit2 and cause use-after-frees :S

and add a test, haha.

wow, I just tried to go through the code paths, and yes it's pretty complicated. My question is, if the pointer in the smartTransportCallback is in fact a copy, and has nothing to do with the original *git_remote, then, we can safely free it right? it won't affect libgit2's cloning process since that will be performed on the original *git_remote pointer. (I am super new to C bindings, and libgit2, so if it sounds stupid, it probably is)

solution 2, is it possible to fix this pointer allocation on libgit2's side? can we submit an upstream PR that returns the same objects that are passed to the cloning process?

wow, I just tried to go through the code paths, and yes it's pretty complicated. My question is, if the pointer in the smartTransportCallback is in fact a copy, and has nothing to do with the original *git_remote, then, we can safely free it right?

we can't ^^;; that pointer is owned by libgit2 and if we free it we will cause a double free D: https://github.com/libgit2/libgit2/blob/5bd49aeeeb7a69f85249b970ac4e2c7011b7127d/src/clone.c#L407-L421 (the last line is the call to git_remote_free on the copy).

it won't affect libgit2's cloning process since that will be performed on the original *git_remote pointer. (I am super new to C bindings, and libgit2, so if it sounds stupid, it probably is)

solution 2, is it possible to fix this pointer allocation on libgit2's side? can we submit an upstream PR that returns the same objects that are passed to the cloning process?

i don't think so, that copy was introduced very intentionally: libgit2/libgit2@3c60768

right, so it landlocks on all sides. What code path does it take when the OpenSSL / libssh2 is compiled against libgit2? cause it seems to work just fine along that code path. Can we try to mimic what happens along that path?

right, so it landlocks on all sides. What code path does it take when the OpenSSL / libssh2 is compiled against libgit2? cause it seems to work just fine along that code path. Can we try to mimic what happens along that path?

we can't: that codepath is done 100% in libgit2 D: anything that crosses the Go/C boundary needs to go through a very different codepath.

What we are working on is OSS by the way, and can be consulted at: fluxcd/source-controller#465 (which depends on v31 targeted work in fluxcd/source-controller#462, but we want to move to v33 as soon as we get it working properly, given C dependencies are resulting in "other issues" (build complexity, etc.)).

nice! I'll take a look for contributing too:)

for this

For HTTPS, no matter what changes I am trying to make, none of the callbacks seem to work (username and password, custom CA, etc.).

this I think I know why, when the code for making the fake remote is executed, data for callbacks from the original remote pointer is lost, I can possibly test my hypothesis and update

can you link a small test in the fluxCD codebase I can use to make a quick and dirty test(with HTTPS and callbacks)?

If you run the pkg/git/strategy test from #465, you'll see it fail.

got it :) let me try to make some changes to the libgit code on my side and see if that test works with my assumed hypothesis

I think your hypothesis is right by the way, but this should either be documented, or properly addressed.

The workaround for us would be to first initialize an empty repository, then add a remote, and then call fetch, but having a working Clone would safe us from a lot of "bootstrapping" :-)

the way I understand the git code (at least the non C parts) is that here

func smartTransportCallback(
	errorMessage **C.char,
	out **C.git_transport,
	owner *C.git_remote,
	handle unsafe.Pointer,
) C.int {
	registeredSmartTransport := pointerHandles.Get(handle).(*RegisteredSmartTransport)
	remote, ok := remotePointers.get(owner)
	if !ok {
		// create a new empty remote and set it
		// as a weak pointer, so that control stays in golang
		remote = createNewEmptyRemote()
		remote.weak = true
	}
...

the issue is since the go code does not get access to the originally created object (which should be the owners arg), I am not sure how to propagate the callbacks you are setting in the code to this bit of code.
There is also a high chance this is not the place to look for the bug 😂
I am trying something here, if it works, Clone will work no workarounds needed

@lhchavez

UPDATES:
I looked at the code, the callbacks are going through C code, so there is a high chance it's not as easy a fix, imo we need a way to recover the original callback funcs ptrs.

@lhchavez can you please assist on how to fix this?

If it is too hard to fix, we'll resort to #836 (comment). But I think this limitation should in this case be documented, so that other people don't fall into the same trap.

makes sense, maybe a small note in the README, but I think this is a major point that needs to be solved cause moving forward, if go transports are to work, they have to work with these callbacks, (I am probably just missing something really small and obvious :P) let's wait for the maintainers/owners to lend their feedback till then

Another error for the mix, I rewrote the callbacks to be more like the git2go tests in: 3290736 (#465)

This has made a new error surface in which the connection simply seems to get closed prematurely:

--- FAIL: TestCheckoutStrategyForImplementation_Auth (0.78s)
    --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_HTTP_clone (0.02s)
        strategy_test.go:63: 
            Unexpected error:
                <*fmt.wrapError | 0xc0002f62e0>: {
                    msg: "unable to clone: Post \"***127.0.0.1:36539/bar/test-reponame/git-upload-pack\": io: read/write on closed pipe",
                    err: <*git.GitError | 0xc0002f62c0>{
                        Message: "Post \"***127.0.0.1:36539/bar/test-reponame/git-upload-pack\": io: read/write on closed pipe",
                        Class: 26,
                        Code: -7,
                    },
                }
                unable to clone: Post "***127.0.0.1:36539/bar/test-reponame/git-upload-pack": io: read/write on closed pipe
            occurred

bugfix: HTTPS Clone fails with remote pointer not found using Go tran…

@hiddeco

@hiddeco did this comment about premature closing get resolved? did you find a workaround?

per: #836 (comment)

We are seeing io: read/write on closed pipe for https

Navigation Menu

Repository navigation

HTTPS `Clone` fails with `remote pointer not found` using Go transport #836

29 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

HTTPS Clone fails with remote pointer not found using Go transport #836

Description

Activity

codexetreme commented on Oct 8, 2021

hiddeco commented on Oct 8, 2021

codexetreme commented on Oct 8, 2021

codexetreme commented on Oct 8, 2021

codexetreme commented on Oct 8, 2021

codexetreme commented on Oct 8, 2021

lhchavez commented on Oct 11, 2021

codexetreme commented on Oct 11, 2021

lhchavez commented on Oct 11, 2021

codexetreme commented on Oct 11, 2021

lhchavez commented on Oct 11, 2021

codexetreme commented on Oct 11, 2021

lhchavez commented on Oct 11, 2021

codexetreme commented on Oct 11, 2021

lhchavez commented on Oct 11, 2021

29 remaining items

codexetreme commented on Oct 25, 2021

codexetreme commented on Oct 25, 2021

hiddeco commented on Oct 25, 2021

codexetreme commented on Oct 25, 2021

hiddeco commented on Oct 25, 2021

codexetreme commented on Oct 25, 2021

codexetreme commented on Oct 25, 2021

hiddeco commented on Oct 25, 2021

codexetreme commented on Oct 25, 2021

hiddeco commented on Oct 25, 2021

tylerphelan commented on Nov 18, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

HTTPS `Clone` fails with `remote pointer not found` using Go transport #836