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

Closed
opened 2021-09-28 16:07:40 -05:00 by hiddeco · 49 comments
hiddeco commented 2021-09-28 16:07:40 -05:00 (Migrated from github.com)

Given:

  1. All tests (even Clone) initialize a Repository before they run a test
  2. The tests that make use of Clone seem to be using a local file path
  3. 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).

Given: 1. All tests (even `Clone`) initialize a `Repository` before they run a test 1. The tests that make use of `Clone` seem to be using a [local file path](https://github.com/libgit2/git2go/blob/main/clone_test.go#L14-L26) 1. 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: ```go 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](https://github.com/libgit2/libgit2/blob/v1.2.0/include/git2/errors.h#L30-L35)).
codexetreme commented 2021-10-08 01:42:06 -05:00 (Migrated from github.com)

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

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
hiddeco commented 2021-10-08 02:30:21 -05:00 (Migrated from github.com)

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.

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.
codexetreme commented 2021-10-08 02:43:22 -05:00 (Migrated from github.com)

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

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
codexetreme commented 2021-10-08 06:50:04 -05:00 (Migrated from github.com)

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

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
codexetreme commented 2021-10-08 07:05:29 -05:00 (Migrated from github.com)

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

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
codexetreme commented 2021-10-08 08:02:56 -05:00 (Migrated from github.com)

@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

@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
lhchavez commented 2021-10-11 09:28:33 -05:00 (Migrated from github.com)

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.

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.
codexetreme commented 2021-10-11 10:24:33 -05:00 (Migrated from github.com)

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 GitHubhttps://github.com/libgit2/git2go/issues/836#issuecomment-940083296, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACCHNM4CNQ73LKSYNNPUK2LUGLYBZANCNFSM5E6KTZ2A.

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<https://github.com/libgit2/git2go/issues/836#issuecomment-940083296>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACCHNM4CNQ73LKSYNNPUK2LUGLYBZANCNFSM5E6KTZ2A>.
lhchavez commented 2021-10-11 10:45:26 -05:00 (Migrated from github.com)

But why does it work with the openssl libraries activated?

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

> But why does it work with the openssl libraries activated? that uses a different codepath that doesn't involve the remote callbacks.
codexetreme commented 2021-10-11 13:34:30 -05:00 (Migrated from github.com)

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.
lhchavez commented 2021-10-11 15:21:49 -05:00 (Migrated from github.com)

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.

> 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.
codexetreme commented 2021-10-11 16:11:23 -05:00 (Migrated from github.com)

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? 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?
lhchavez commented 2021-10-11 16:16:34 -05:00 (Migrated from github.com)

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: 5bd49aeeeb/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: 3c607685da

> 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](https://owasp.org/www-community/vulnerabilities/Doubly_freeing_memory) 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: https://github.com/libgit2/libgit2/commit/3c607685da2cf9a22559f53fd7670e1c4cad45e4
codexetreme commented 2021-10-11 16:21:02 -05:00 (Migrated from github.com)

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?
lhchavez commented 2021-10-11 16:22:31 -05:00 (Migrated from github.com)

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.

> 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.
codexetreme commented 2021-10-11 16:25:15 -05:00 (Migrated from github.com)

right, if you don't mind, can you explain what code path is taken for the Go/C cloning process? I lose it the minute it goes to C.git_clone(args). Perhaps, then I can spend some time to think about this, and maybe try to make an educated guess and propose a solution. Cause I'm all out of ideas rn

right, if you don't mind, can you explain what code path is taken for the Go/C cloning process? I lose it the minute it goes to C.git_clone(args). Perhaps, then I can spend some time to think about this, and maybe try to make an educated guess and propose a solution. Cause I'm all out of ideas rn
lhchavez commented 2021-10-11 16:37:58 -05:00 (Migrated from github.com)

right, if you don't mind, can you explain what code path is taken for the Go/C cloning process? I lose it the minute it goes to C.git_clone(args). Perhaps, then I can spend some time to think about this, and maybe try to make an educated guess and propose a solution. Cause I'm all out of ideas rn

yeah, if OpenSSL / libssh2 are involved, that's pretty much it: once it goes to C.git_clone(args), there's nothing else observable from Go. once it returns, the clone is finished (or failed).

when this goes through Go, b7bad55e4b/src/clone.c (L348) calls 9b155184fe/clone.go (L56) to create the Remote, and then there's some more shenanigans with the Transports. f1fa96c7b7 is a good place to look at how the transports themselves are plumbed through and then b983e1daeb outlines the HTTPS-only part.

> right, if you don't mind, can you explain what code path is taken for the Go/C cloning process? I lose it the minute it goes to C.git_clone(args). Perhaps, then I can spend some time to think about this, and maybe try to make an educated guess and propose a solution. Cause I'm all out of ideas rn yeah, if OpenSSL / libssh2 are involved, that's pretty much it: once it goes to `C.git_clone(args)`, there's nothing else observable from Go. once it returns, the clone is finished (or failed). when this goes through Go, https://github.com/libgit2/libgit2/blob/b7bad55e4bb0a285b073ba5e02b01d3f522fc95d/src/clone.c#L348 calls https://github.com/libgit2/git2go/blob/9b155184fe1f0f0258730bccd2759ac7ec39be2a/clone.go#L56 to create the `Remote`, and then there's some more shenanigans with the Transports. https://github.com/libgit2/git2go/commit/f1fa96c7b7f548389c7560d3a1a0bce83be56c9f is a good place to look at how the transports themselves are plumbed through and then https://github.com/libgit2/git2go/commit/b983e1daebf528443e2a3954cd595fa3664ec93f outlines the HTTPS-only part.
codexetreme commented 2021-10-11 16:41:58 -05:00 (Migrated from github.com)

thanks for the pointers :P. I will trace this and try to think of a solution.

thanks for the pointers :P. I will trace this and try to think of a solution.
lhchavez commented 2021-10-11 16:44:39 -05:00 (Migrated from github.com)

thanks for the pointers :P. I will trace this and try to think of a solution.

sounds good! https://github.com/libgit2/git2go/issues/836#issuecomment-940414675 contains a partial outline of what the solution might end up looking like.

> thanks for the pointers :P. I will trace this and try to think of a solution. sounds good! https://github.com/libgit2/git2go/issues/836#issuecomment-940414675 contains a partial outline of what the solution might end up looking like.
codexetreme commented 2021-10-13 05:22:13 -05:00 (Migrated from github.com)

I followed the chain, from what I can see, the callback is only triggered when it is explicitly passed.
1697cd6ff5/src/clone.c (L272-L278)

I trace it further to

b7bad55e4b/src/remote.c (L767)

now just to check if my working theory is on track,
b7bad55e4b/src/remote.c (L794-L798)

these lines will trigger the go transport callbacks, right?

so, let's say we run a simple clone code from golang, and don't pass any callback funcs for remoteCallbacks, it will try to use the native C default callbacks and invoke the HTTPS C transports.

But if we do pass the callbacks in golang, it will trigger those instead, thus now bringing the transports control back in golang.

If this working theory is incorrect please let me know. I think if I understand this right, I might get closer to a solution

EDIT: I am trying to understand most of this code from libgit2 master and the version linked in the git2go submodule so there is a slight chance the code links may not perfectly match up

I followed the chain, from what I can see, the callback is only triggered when it is explicitly passed. https://github.com/libgit2/libgit2/blob/1697cd6ff5d29c95106ff4b7bd56ebba5d51b8c1/src/clone.c#L272-L278 I trace it further to https://github.com/libgit2/libgit2/blob/b7bad55e4bb0a285b073ba5e02b01d3f522fc95d/src/remote.c#L767 now just to check if my working theory is on track, https://github.com/libgit2/libgit2/blob/b7bad55e4bb0a285b073ba5e02b01d3f522fc95d/src/remote.c#L794-L798 these lines will trigger the go transport callbacks, right? so, let's say we run a simple clone code from golang, and don't pass any callback funcs for remoteCallbacks, it will try to use the native C default callbacks and invoke the HTTPS C transports. But if we do pass the callbacks in golang, it will trigger those instead, thus now bringing the transports control back in golang. If this working theory is incorrect please let me know. I think if I understand this right, I might get closer to a solution EDIT: I am trying to understand most of this code from libgit2 master and the version linked in the git2go submodule so there is a slight chance the code links may not perfectly match up
lhchavez commented 2021-10-13 08:54:33 -05:00 (Migrated from github.com)

so, let's say we run a simple clone code from golang, and don't pass any callback funcs for remoteCallbacks, it will try to use the native C default callbacks and invoke the HTTPS C transports.

it will try to use the native C default callbacks for remotes, but transports are a completely different beast that use different codepaths: If the remote object already has a transport assigned to it or the remote callbacks have a transport factory, it will use those. otherwise, it will use git_transport_new, which has its own set of callbacks. git2go registers the transport callbacks upon library initialization.

But if we do pass the callbacks in golang, it will trigger those instead, thus now bringing the transports control back in golang.

control can be passed back to golang even without the remote callbacks: it can also be returned when the transports callbacks are registered.

If this working theory is incorrect please let me know. I think if I understand this right, I might get closer to a solution

the outline of the solution in https://github.com/libgit2/git2go/issues/836#issuecomment-940414675 is more or less as follows:

  • Modify Remote to add a flag (called weak for consistency with the one from Repository) that tells Remote.Free to not call git_remote_free to prevent the double-free.
  • if we can't find the git_remote * by pointer in the list of tracked pointers, a new Remote object can be created with the weak flag turned on. The newly-created Remote's callbacks can be left nil because if it hadn't been nil in the original object, we would have found it in the list of remotes!

But there's something that I didn't think of beforehand: We would also need to create a Repository object from the git_remote_owner of the git_remote *, so we might have to also track the mapping of all the Repository/git_repository * weak objects that we know of. f1fa96c7b7 has the code to track Remote objects, so maybe a similar pattern can be used for weak Repository objects. That way if (for whatever reason) the user modified the Repository object in the remote callbacks, we can restore that state in the transport callbacks.

> so, let's say we run a simple clone code from golang, and don't pass any callback funcs for remoteCallbacks, it will try to use the native C default callbacks and invoke the HTTPS C transports. it will try to use the native C default callbacks for remotes, but transports are a completely different beast that use different codepaths: If the remote object [already has a transport assigned to it](https://github.com/libgit2/libgit2/blob/b7bad55e4bb0a285b073ba5e02b01d3f522fc95d/src/remote.c#L789) _or_ the remote callbacks have a [transport factory](https://github.com/libgit2/libgit2/blob/b7bad55e4bb0a285b073ba5e02b01d3f522fc95d/src/remote.c#L794-L798), it will use those. otherwise, it will use [`git_transport_new`](https://github.com/libgit2/libgit2/blob/508361401fbb5d87118045eaeae3356a729131aa/src/transport.c#L118-L139), which has its own set of callbacks. git2go registers the transport callbacks upon [library initialization](https://github.com/libgit2/git2go/blob/9b155184fe1f0f0258730bccd2759ac7ec39be2a/git.go#L153-L168). > But if we do pass the callbacks in golang, it will trigger those instead, thus now bringing the transports control back in golang. control can be passed back to golang even without the remote callbacks: it can also be returned when the transports callbacks are registered. > If this working theory is incorrect please let me know. I think if I understand this right, I might get closer to a solution the outline of the solution in https://github.com/libgit2/git2go/issues/836#issuecomment-940414675 is more or less as follows: * Modify `Remote` to add a flag (called `weak` for consistency with the one from [`Repository`](https://github.com/libgit2/git2go/blob/9b155184fe1f0f0258730bccd2759ac7ec39be2a/repository.go#L40-L42)) that tells `Remote.Free` to _not_ call `git_remote_free` to prevent the double-free. * if we can't find the `git_remote *` by pointer in [the list of tracked pointers](https://github.com/libgit2/git2go/blob/dcc9331226b5ec340fe4cf7fa3f6b5188d8779e9/transport.go#L308-L311), a new `Remote` object can be created with the `weak` flag turned on. The newly-created `Remote`'s `callbacks` can be left `nil` because if it hadn't been `nil` in the original object, we would have found it in the list of remotes! But there's something that I didn't think of beforehand: We would also need to create a `Repository` object from the [`git_remote_owner`](https://libgit2.org/libgit2/#HEAD/group/remote/git_remote_owner) of the `git_remote *`, so we _might_ have to also track the mapping of all the `Repository`/`git_repository *` weak objects that we know of. https://github.com/libgit2/git2go/commit/f1fa96c7b7f548389c7560d3a1a0bce83be56c9f has the code to track `Remote` objects, so maybe a similar pattern can be used for weak `Repository` objects. That way if (for whatever reason) the user modified the `Repository` object in the remote callbacks, we can restore that state in the transport callbacks.
codexetreme commented 2021-10-13 11:37:33 -05:00 (Migrated from github.com)

Right with this, I think can start work to get a pr opened. Give me a few days to get this up. I'm now quite clear on what's happening to a large degree

Right with this, I think can start work to get a pr opened. Give me a few days to get this up. I'm now quite clear on what's happening to a large degree
lhchavez commented 2021-10-14 05:58:15 -05:00 (Migrated from github.com)

update! i just realized that the Repository objects are completely stateless in git2go: all state is stored in the libgit2 layer. so we don't need to track Repository objects at all! just create a new weak one and be done with it :D

update! i just realized that the `Repository` objects are completely stateless in git2go: all state is stored in the libgit2 layer. so we don't need to track `Repository` objects at all! just create a new weak one and be done with it :D
codexetreme commented 2021-10-14 06:01:12 -05:00 (Migrated from github.com)

This is good to know. Cause I'll be honest. I'm thinking I've gone in over my head, trying to create a pr. Now that there is less changes to make it is always good :P

This is good to know. Cause I'll be honest. I'm thinking I've gone in over my head, trying to create a pr. Now that there is less changes to make it is always good :P
codexetreme commented 2021-10-15 03:43:45 -05:00 (Migrated from github.com)

I have created a PR, I think this is the bare minimum change like you listed out, I will add tests for this (I need a valid GitHub url that I can add in the test, will this repo's link itself be okay?)
Plus fixing / adding any more code/comments/formatting too

I have created a PR, I think this is the bare minimum change like you listed out, I will add tests for this (I need a valid GitHub url that I can add in the test, will this repo's link itself be okay?) Plus fixing / adding any more code/comments/formatting too
hiddeco commented 2021-10-15 03:55:58 -05:00 (Migrated from github.com)

@codexetreme bit of unsolicited advice - based on experience relying on real GitHub repositories for tests (and moving away from them) - I would use a mocking Git server using a project like https://github.com/sosedoff/gitkit

In addition, thank you very much for taking care of this, as I was out of time 🙇 🥇

@codexetreme bit of unsolicited advice - based on experience relying on real GitHub repositories for tests (and moving away from them) - I would use a mocking Git server using a project like https://github.com/sosedoff/gitkit In addition, thank you very much for taking care of this, as I was out of time :bow: :1st_place_medal:
codexetreme commented 2021-10-15 04:06:53 -05:00 (Migrated from github.com)

ah I thought so too. Just pushed a test where I was checking if github was up :P.

in anycase, your(@lhchavez) solution works !!!!!!
test clones (as of yet github repo) via https no questions asked :)

I will get the mocking server up in the test code now

ah I thought so too. Just pushed a test where I was checking if github was up :P. in anycase, your(@lhchavez) solution works !!!!!! test clones (as of yet github repo) via https no questions asked :) I will get the mocking server up in the test code now
codexetreme commented 2021-10-15 05:13:13 -05:00 (Migrated from github.com)

okay, so the mocking server has no HTTPS support, which means, we'll need to add it externally via some other mechanism for the test (since the point is to test HTTPS transport :P)

EDIT: any suggestions/ideas?

okay, so the mocking server has no HTTPS support, which means, we'll need to add it externally via some other mechanism for the test (since the point is to test HTTPS transport :P) EDIT: any suggestions/ideas?
hiddeco commented 2021-10-15 05:28:10 -05:00 (Migrated from github.com)

The library I mentioned has support for HTTP/S, but you have to create some tiny wrappers to make it work, see https://github.com/fluxcd/pkg/blob/main/gittestserver/server.go#L134 for our take :-).

The library I mentioned has support for HTTP/S, but you have to create some tiny wrappers to make it work, see https://github.com/fluxcd/pkg/blob/main/gittestserver/server.go#L134 for our take :-).
lhchavez commented 2021-10-15 08:17:06 -05:00 (Migrated from github.com)

no need for HTTPS in the test! tests always use the managed transports: 6eae74c128/git_test.go (L14)

HTTP-only is fine (even preferred) to keep this being simpler.

no need for HTTPS in the test! tests always use the managed transports: https://github.com/libgit2/git2go/blob/6eae74c128e37e590d3a5da3fe0561add4b94d5c/git_test.go#L14 HTTP-only is fine (even preferred) to keep this being simpler.
codexetreme commented 2021-10-15 09:12:14 -05:00 (Migrated from github.com)

Ah, so there's no need to reference a repo on GitHub/gitlab to test this?

Also, can you share your Dev environment setup? I'm currently opening the project on intellij with the go plugin. This works for the most part, but misses some cgo calls as errors

Ah, so there's no need to reference a repo on GitHub/gitlab to test this? Also, can you share your Dev environment setup? I'm currently opening the project on intellij with the go plugin. This works for the most part, but misses some cgo calls as errors
lhchavez commented 2021-10-16 07:50:28 -05:00 (Migrated from github.com)

Ah, so there's no need to reference a repo on GitHub/gitlab to test this?

@hiddeco 's suggestion is to have a fully hermetic test that does not require communicating to any external server over the network, and only has a dependency on the git binary being installed on the test machine (a reasonable assumption).

Also, can you share your Dev environment setup? I'm currently opening the project on intellij with the go plugin. This works for the most part, but misses some cgo calls as errors

ha, i have a much more spartan setup: Vim with https://github.com/dense-analysis/ale and https://pkg.go.dev/golang.org/x/tools/gopls . cgo in general is hard to analyze :S

> Ah, so there's no need to reference a repo on GitHub/gitlab to test this? @hiddeco 's suggestion is to have a fully hermetic test that does not require communicating to any external server over the network, and only has a dependency on the `git` binary being installed on the test machine (a reasonable assumption). > Also, can you share your Dev environment setup? I'm currently opening the project on intellij with the go plugin. This works for the most part, but misses some cgo calls as errors ha, i have a _much_ more spartan setup: Vim with https://github.com/dense-analysis/ale and https://pkg.go.dev/golang.org/x/tools/gopls . cgo in general is hard to analyze :S
codexetreme commented 2021-10-18 12:49:49 -05:00 (Migrated from github.com)

Hello, so I'm still a bit confused. (My sincere apologies , since I'm still not 100% sure about what the entire codebase does).

The server we'll need to setup for this test, do we add it as a dependency in go.mod (it's just one test, so I wonder).

When loading the repo into the server, do we reference the locally cloned repo from which the test is being run ? (I'm not sure if this is entirely possible, but seems doable)

With managed transports, why do we prefer it being in http? In my experience, Https is the one that messes up all the time in any communication ( SAN domains, certs, cipher suites etc, etc)

Considering all this, locally I have a simple setup which runs the server with this repo cloned in a separate location. Then I give it as a http URL to this test.

Can you please offer further insight and guidance?

Hello, so I'm still a bit confused. (My sincere apologies , since I'm still not 100% sure about what the entire codebase does). The server we'll need to setup for this test, do we add it as a dependency in go.mod (it's just one test, so I wonder). When loading the repo into the server, do we reference the locally cloned repo from which the test is being run ? (I'm not sure if this is entirely possible, but seems doable) With managed transports, why do we prefer it being in http? In my experience, Https is the one that messes up all the time in any communication ( SAN domains, certs, cipher suites etc, etc) Considering all this, locally I have a simple setup which runs the server with this repo cloned in a separate location. Then I give it as a http URL to this test. Can you please offer further insight and guidance?
codexetreme commented 2021-10-20 13:43:27 -05:00 (Migrated from github.com)

UPDATES:

I took some time to rethink and here's what I've come up with:

  1. using the pre-existing tests code I now create an empty dummy repo
  2. use the gitkit server and some nice helper functions from @hiddeco's file link above
  3. clone said repo in the test and fail it if clone fails

now there is no dependency on external networks or any confusion with repos and such

please let me know if I've missed something :)

UPDATES: I took some time to rethink and here's what I've come up with: 1. using the pre-existing tests code I now create an empty dummy repo 2. use the gitkit server and some nice helper functions from @hiddeco's file link above 3. clone said repo in the test and fail it if clone fails now there is no dependency on external networks or any confusion with repos and such please let me know if I've missed something :)
hiddeco commented 2021-10-25 12:09:08 -05:00 (Migrated from github.com)

Is it possible that during this fix, a detail around the FetchOptions#RemoteCallbacks was forgotten? Looking at the tests, this should work in combination with calling Fetch:

However, our code which makes use of Clone and has test coverage for various authentication configurations, fails if we attempt to update from v31 (with OpenSSL and OpenSSH2) to v33 (without any additional C dependencies):

ok      github.com/fluxcd/source-controller/pkg/git     (cached)
ok      github.com/fluxcd/source-controller/pkg/git/gogit       (cached)
ok      github.com/fluxcd/source-controller/pkg/git/libgit2     (cached)
...
--- FAIL: TestCheckoutStrategyForImplementation_Auth (0.70s)
    --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_http_cloning (0.01s)
        strategy_test.go:63: 
            Unexpected error:
                <*fmt.wrapError | 0xc0001be080>: {
                    msg: "unable to clone 'http://test-user:test-password@127.0.0.1:38903/bar/test-reponame', error: credential is not userpass plaintext",
                    err: <*git.GitError | 0xc0001be060>{
                        Message: "credential is not userpass plaintext",
                        Class: 26,
                        Code: -7,
                    },
                }
                unable to clone 'http://test-user:test-password@127.0.0.1:38903/bar/test-reponame', error: credential is not userpass plaintext
            occurred
    --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_https_cloning (0.01s)
        strategy_test.go:82: 
            Unexpected error:
                <*fmt.wrapError | 0xc0004781a0>: {
                    msg: "unable to clone 'https://127.0.0.1:43273/bar/test-reponame', error: Get \"https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack\": x509: certificate signed by unknown authority",
                    err: <*git.GitError | 0xc000478180>{
                        Message: "Get \"https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack\": x509: certificate signed by unknown authority",
                        Class: 26,
                        Code: -7,
                    },
                }
                unable to clone 'https://127.0.0.1:43273/bar/test-reponame', error: Get "https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack": x509: certificate signed by unknown authority
            occurred
    --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_ssh_cloning (0.31s)
        strategy_test.go:109: 
            Unexpected error:
                <*fmt.wrapError | 0xc0002da280>: {
                    msg: "unable to clone 'ssh://git@localhost:37157/bar/test-reponame', error: this version of libgit2 was not built with ssh memory credentials.",
                    err: <*git.GitError | 0xc0002da260>{
                        Message: "this version of libgit2 was not built with ssh memory credentials.",
                        Class: 26,
                        Code: -1,
                    },
                }
                unable to clone 'ssh://git@localhost:37157/bar/test-reponame', error: this version of libgit2 was not built with ssh memory credentials.
            occurred
FAIL
FAIL    github.com/fluxcd/source-controller/pkg/git/strategy    0.709s
FAIL
Is it possible that during this fix, a detail around the `FetchOptions#RemoteCallbacks` was forgotten? Looking at the tests, this should work in combination with calling `Fetch`: - https://github.com/libgit2/git2go/blob/main/remote_test.go#L446 - https://github.com/libgit2/git2go/blob/main/remote_test.go#L50 However, our code which makes use of `Clone` and has test coverage for various authentication configurations, fails if we attempt to update from `v31` (with OpenSSL and OpenSSH2) to `v33` (without any additional C dependencies): ``` ok github.com/fluxcd/source-controller/pkg/git (cached) ok github.com/fluxcd/source-controller/pkg/git/gogit (cached) ok github.com/fluxcd/source-controller/pkg/git/libgit2 (cached) ... --- FAIL: TestCheckoutStrategyForImplementation_Auth (0.70s) --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_http_cloning (0.01s) strategy_test.go:63: Unexpected error: <*fmt.wrapError | 0xc0001be080>: { msg: "unable to clone 'http://test-user:test-password@127.0.0.1:38903/bar/test-reponame', error: credential is not userpass plaintext", err: <*git.GitError | 0xc0001be060>{ Message: "credential is not userpass plaintext", Class: 26, Code: -7, }, } unable to clone 'http://test-user:test-password@127.0.0.1:38903/bar/test-reponame', error: credential is not userpass plaintext occurred --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_https_cloning (0.01s) strategy_test.go:82: Unexpected error: <*fmt.wrapError | 0xc0004781a0>: { msg: "unable to clone 'https://127.0.0.1:43273/bar/test-reponame', error: Get \"https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack\": x509: certificate signed by unknown authority", err: <*git.GitError | 0xc000478180>{ Message: "Get \"https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack\": x509: certificate signed by unknown authority", Class: 26, Code: -7, }, } unable to clone 'https://127.0.0.1:43273/bar/test-reponame', error: Get "https://127.0.0.1:43273/bar/test-reponame/info/refs?service=git-upload-pack": x509: certificate signed by unknown authority occurred --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_ssh_cloning (0.31s) strategy_test.go:109: Unexpected error: <*fmt.wrapError | 0xc0002da280>: { msg: "unable to clone 'ssh://git@localhost:37157/bar/test-reponame', error: this version of libgit2 was not built with ssh memory credentials.", err: <*git.GitError | 0xc0002da260>{ Message: "this version of libgit2 was not built with ssh memory credentials.", Class: 26, Code: -1, }, } unable to clone 'ssh://git@localhost:37157/bar/test-reponame', error: this version of libgit2 was not built with ssh memory credentials. occurred FAIL FAIL github.com/fluxcd/source-controller/pkg/git/strategy 0.709s FAIL ```
hiddeco commented 2021-10-25 12:23:55 -05:00 (Migrated from github.com)

Ah, looks like the SSH issue would be solved by switching to NewCredentialSSHKeyFromSigner, the certificate issue however I still can't explain.

Ah, looks like the SSH issue would be solved by switching to `NewCredentialSSHKeyFromSigner`, the certificate issue however I still can't explain.
codexetreme commented 2021-10-25 13:11:14 -05:00 (Migrated from github.com)

once you switch to NewCredentialSSHKeyFromSigner can you post the new error? I wanna see what the new error is,
just wanna try and fix (still relatively new to OSS, so yknow might be slow, but eventually I'll get there 😅 )

plus, I am going to use this lib in my own personal project too, and your use case for using this lib is the similar to mine :P, any errors you face, I will face too

once you switch to `NewCredentialSSHKeyFromSigner` can you post the new error? I wanna see what the new error is, just wanna try and fix (still relatively new to OSS, so yknow might be slow, but eventually I'll get there :sweat_smile: ) plus, I am going to use this lib in my own personal project too, and your use case for using this lib is the similar to mine :P, any errors you face, I will face too
hiddeco commented 2021-10-25 13:45:14 -05:00 (Migrated from github.com)

The NewCredentialSSHKeyFromSigner changed something for SSH, but still didn't result in a working transport:

    --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_ssh_cloning (0.24s)
        strategy_test.go:109: 
            Unexpected error:
                <*fmt.wrapError | 0xc0002bc6c0>: {
                    msg: "unable to clone 'ssh://git@localhost:46041/bar/test-reponame', error: EOF",
                    err: <*git.GitError | 0xc0002bc6a0>{Message: "EOF", Class: 26, Code: -7},
                }
                unable to clone 'ssh://git@localhost:46041/bar/test-reponame', error: EOF

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

What we are working on is OSS by the way, and can be consulted at: https://github.com/fluxcd/source-controller/pull/465 (which depends on v31 targeted work in https://github.com/fluxcd/source-controller/pull/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.)).

The `NewCredentialSSHKeyFromSigner` changed something for SSH, but still didn't result in a working transport: ``` --- FAIL: TestCheckoutStrategyForImplementation_Auth/libgit2_ssh_cloning (0.24s) strategy_test.go:109: Unexpected error: <*fmt.wrapError | 0xc0002bc6c0>: { msg: "unable to clone 'ssh://git@localhost:46041/bar/test-reponame', error: EOF", err: <*git.GitError | 0xc0002bc6a0>{Message: "EOF", Class: 26, Code: -7}, } unable to clone 'ssh://git@localhost:46041/bar/test-reponame', error: EOF ``` For HTTPS, no matter what changes I am trying to make, none of the callbacks seem to work (username and password, custom CA, etc.). What we are working on is OSS by the way, and can be consulted at: https://github.com/fluxcd/source-controller/pull/465 (which depends on `v31` targeted work in https://github.com/fluxcd/source-controller/pull/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.)).
codexetreme commented 2021-10-25 14:04:00 -05:00 (Migrated from github.com)

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

>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
codexetreme commented 2021-10-25 14:06:51 -05:00 (Migrated from github.com)

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

can you link a small test in the fluxCD codebase I can use to make a quick and dirty test(with HTTPS and callbacks)?
hiddeco commented 2021-10-25 14:09:41 -05:00 (Migrated from github.com)

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

If you run the `pkg/git/strategy` test from `#465`, you'll see it fail.
codexetreme commented 2021-10-25 14:10:34 -05:00 (Migrated from github.com)

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

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
hiddeco commented 2021-10-25 14:13:58 -05:00 (Migrated from github.com)

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" :-)

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" :-)
codexetreme commented 2021-10-25 14:39:04 -05:00 (Migrated from github.com)

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

the way I understand the git code (at least the non C parts) is that here ```go 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 :joy: I am trying something here, if it works, `Clone` will work no workarounds needed
codexetreme commented 2021-10-25 15:04:13 -05:00 (Migrated from github.com)

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?

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?
hiddeco commented 2021-10-25 15:10:56 -05:00 (Migrated from github.com)

If it is too hard to fix, we'll resort to https://github.com/libgit2/git2go/issues/836#issuecomment-951228416. But I think this limitation should in this case be documented, so that other people don't fall into the same trap.

If it is too hard to fix, we'll resort to https://github.com/libgit2/git2go/issues/836#issuecomment-951228416. But I think this limitation should in this case be documented, so that other people don't fall into the same trap.
codexetreme commented 2021-10-25 15:15:22 -05:00 (Migrated from github.com)

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

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
hiddeco commented 2021-10-25 15:25:34 -05:00 (Migrated from github.com)

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
Another error for the mix, I rewrote the callbacks to be more like the git2go tests in: [`3290736` (#465)](https://github.com/fluxcd/source-controller/pull/465/commits/329073660693e356c7c5c37ba2be9226a42aa2d8) 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 ````
tylerphelan commented 2022-11-18 11:44:30 -06:00 (Migrated from github.com)

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

per: https://github.com/libgit2/git2go/issues/836#issuecomment-951291223

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

@hiddeco did this comment about premature closing get resolved? did you find a workaround? per: https://github.com/libgit2/git2go/issues/836#issuecomment-951291223 We are seeing `io: read/write on closed pipe` for https
Sign in to join this conversation.
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: jcarr/git2go#836
No description provided.