larshp / abapGitServer

Git server implemented in ABAP
MIT License
64 stars 25 forks source link

Unexpected unpack status when using abapGit #156

Open pokrakam opened 3 years ago

pokrakam commented 3 years ago

Getting the following error when committing with abapGit: Git protocol error: Unexpected unpack status

Comparing GitHub and AGS, there is a difference in response. When committing to GitHub, I get the response:

000eunpack ok
0017ok refs/heads/main
00000000

but committing to AGS I get:

0013#000eunpack ok
001e#0019ok refs/heads/master
0009#00000000

Looks like the length header is added twice by AGS?

pokrakam commented 3 years ago

Hmm, so been learning about Git protocol bands. The header may be correct if it's shown raw. Not sure which side the issue is. Either it's doubling up the band in AGS or not stripping it out in abapGit.

See also https://github.com/abapGit/abapGit/issues/4872

mbtools commented 3 years ago

i'm not using AGS, but I would say the error is here: https://github.com/larshp/abapGitServer/blob/main/src/service/zcl_ags_service_git.clas.abap#L488

Suggestion:

    lo_response->append_length( zcl_ags_util=>string_to_xstring_utf8( |unpack ok\n| ) ).

    lo_response->append_length( zcl_ags_util=>string_to_xstring_utf8( |ok { iv_branch_name }\n| ) ).

    lo_response->append_length( zcl_ags_util=>string_to_xstring_utf8( '0000' ) ).

    rv_xstring = lo_response->get( ).
pokrakam commented 3 years ago

Have debugged that and it is definitely correct in adding the band. The other unusual thing is that I can pull from CLI, which makes me wonder if it's not AG. The protocol seems to agree with the docs too.

Mind you, I didn't try CLI with the latest version, might do that when I get some time.

mbtools commented 3 years ago

As far as I see it, AG (client) does not set the side-band when checking report-status (it does for upload_pack): https://github.com/abapGit/abapGit/blob/main/src/git/zcl_abapgit_git_transport.clas.abap#L288 Therefore, AG server should not send multiplexed data.

To support also other Git clients, AGS would need to be enhanced for both situations (i.e. evaluate header, use current code if side-band was requested, use my code otherwise).

pokrakam commented 3 years ago

Perhaps, or AG should handle it as it did before. That's why I logged it in both.

In any case I don't have time to do this myself right now. I'm using AGS for a PoC, and the simple workaround is to revert AG to where it still worked.

AndreaBorgia-Abo commented 1 year ago

@pokrakam Oh, well, nice to see I am not the only one: thing is, I wanted to use aGS instead of GH, because reasons, and it seemed a quicker option than setting up an internal git server. I'll discuss with colleagues if we can spend time on this issue.

pokrakam commented 1 year ago

@AndreaBorgia-Abo as this is turning out to be a bit of rabbit hole that I don't have time for and it's still unclear where it should be addressed, I've moved onto running a Gitea image. Really easy to set up.

AndreaBorgia-Abo commented 1 year ago

I believe you, we had to explore this option and this was something I could do (mostly) without external help. Thanks for the suggestion, I'll bring it up.