feat: allow multiple flushes when decoding report status #1597

UrosSimovic · 2025-07-01T14:07:26Z

More info about the problem I tried to fix can be found in the #1571.

Does this make sense? Is there any particular reason we want to stop reading on after receiving first flush?

UrosSimovic · 2025-07-01T14:09:36Z

plumbing/protocol/packp/report_status.go

-	if err != nil && !errors.Is(err, io.EOF) {
-		// TODO: We should not ignore EOF errors here. Decoding a report-status
-		// message ends with a flush-pkt, an EOF indicates that the flush-pkt
-		// was not received.


I guess that TODO was already done before as you check if flush was received and return error if not.

UrosSimovic · 2025-07-01T14:21:18Z

Now that I think it should be probably made optional based on progress is provided or not.

aymanbagabas · 2025-07-07T02:18:14Z

Hi @UrosSimovic, thank you for starting this discussion and opening this issue. The report-status implementation just follows the documentation here https://git-scm.com/docs/gitprotocol-pack#_report_status which expects one flush packet to end the request. I would investigate this more to see if this is related to something else like sideband, or related to GitLab server implementation, and would also look into how canonical Git handles this situation.

UrosSimovic · 2025-07-07T12:34:22Z

@aymanbagabas thank you for the reply. Just ftr, I don't know a lot about git internals, just went deep dive with this PR.

If I get this correctly, report status works just fine. But the problem is in whatever git remote sends afterwards.

By canonical Git you mean vanilla git? I did indeed tested GitLab, GitHub, and local git with dummy echo in post receive script. And only one that works is GitHub, as it sends single flush only at the very end, where both gitlab and local git are sending first flush after report status, and another one at the end where various additional remote messages could be (e.g. link to open gitlab merge request)

The whole reading is done the method I touched and sending to sideband demuxer. That why I went down here. And it works. When I provide a progress in remote.Push I get the the same remote as I'd see when pushing with git cli.

aymanbagabas · 2025-07-07T22:16:20Z

Looking deeper into this, I found a bug in the SendPack logic mainly around the Sideband stuff. Furthermore, for some reason, GitLab doesn't reply with any server-side messages when I use GoGit CLI.

You can notice that GitLab does not send their post-receive server updates when the client is Go-git. GitHub works find and we get the same result for either client. Needs further investigation :/

I've opened #1604 and go-git/cli#14 to fix the issues I found debugging this.

git cli (gitlab):

18:03:51.587473 git.c:476               trace: built-in: git push gl branch22
18:03:51.589269 run-command.c:667       trace: run_command: unset GIT_PREFIX; ssh git@gitlab.com 'git-receive-pack '\''aymanbagabas/dummy.git'\'''
18:03:51.589333 run-command.c:759       trace: start_command: /usr/bin/ssh git@gitlab.com 'git-receive-pack '\''aymanbagabas/dummy.git'\'''
18:03:52.338224 pkt-line.c:85           packet:         push< 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\0report-status report-status-v2 delete-refs side-band-64k quiet atomic ofs-delta push-options object-format=sha1 agent=git/2.49.0.gl2-Linux
18:03:52.338387 pkt-line.c:85           packet:         push< b0dc1ca52c9960e585b5807ec4e876b8768014ba refs/heads/master
18:03:52.338410 pkt-line.c:85           packet:         push< b988a73903336f8c35f0f5177d2ca33ddc730ce1 refs/heads/newbbb
18:03:52.338422 pkt-line.c:85           packet:         push< 1e1bd0038f6cb55321f83f19eb84139daa5c2027 refs/heads/newbranch
18:03:52.338436 pkt-line.c:85           packet:         push< 0000
18:03:52.338781 pkt-line.c:85           packet:         push> 0000000000000000000000000000000000000000 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/branch22\0 report-status-v2 side-band-64k object-format=sha1 agent=git/2.49.0-Darwin
18:03:52.338876 pkt-line.c:85           packet:         push> 0000
18:03:52.339038 run-command.c:667       trace: run_command: git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
18:03:52.339153 run-command.c:759       trace: start_command: /nix/store/ir2bhb7n3ngh0dg7p6jbhm1wa4d8gkb7-git-2.49.0/libexec/git-core/git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
18:03:52.351635 git.c:476               trace: built-in: git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
18:03:52.774513 pkt-line.c:85           packet:     sideband< \1000eunpack ok001bok refs/heads/branch220000
18:03:52.774611 pkt-line.c:85           packet:         push< unpack ok
18:03:52.774627 pkt-line.c:85           packet:         push< ok refs/heads/branch22
18:03:52.774640 pkt-line.c:85           packet:         push< 0000
18:03:52.959975 pkt-line.c:85           packet:     sideband< \2To create a merge request for branch22, visit:  https://gitlab.com/aymanbagabas/dummy/-/merge_requests/new?merge_request%5Bsou
remote:
remote: To create a merge request for branch22, visit:
18:03:52.960047 pkt-line.c:85           packet:     sideband< \2rce_branch%5D=branch22
remote:   https://gitlab.com/aymanbagabas/dummy/-/merge_requests/new?merge_request%5Bsource_branch%5D=branch22
remote:
18:03:52.961402 pkt-line.c:85           packet:     sideband< 0000
To gitlab.com:aymanbagabas/dummy.git
 * [new branch]                branch22 -> branch22

Go-git cli:

17:59:39.881303 pktline.go:233: packet: < 00c8 "74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\x00report-status report-status-v2 delete-refs side-band-64k quiet atomic ofs-delta push-options object-format=sha1 agent=git/2.49.0.gl2-Linux\n"
17:59:39.881495 pktline.go:233: packet: < 00c8 "74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\x00report-status report-status-v2 delete-refs side-band-64k quiet atomic ofs-delta push-options object-format=sha1 agent=git/2.49.0.gl2-Linux\n"
17:59:39.881587 pktline.go:233: packet: < 003f "b0dc1ca52c9960e585b5807ec4e876b8768014ba refs/heads/master\n"
17:59:39.881603 pktline.go:233: packet: < 003f "b988a73903336f8c35f0f5177d2ca33ddc730ce1 refs/heads/newbbb\n"
17:59:39.881614 pktline.go:233: packet: < 0042 "1e1bd0038f6cb55321f83f19eb84139daa5c2027 refs/heads/newbranch\n"
17:59:39.881621 pktline.go:132: packet: < 0000
17:59:39.887066 pktline.go:233: packet: > 0093 "0000000000000000000000000000000000000000 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/branch22\x00report-status side-band-64k agent=git/6.x"
17:59:39.887141 pktline.go:72: packet: > 0000
17:59:40.271516 pktline.go:233: packet: < 0032 "\x01000eunpack ok\n001bok refs/heads/branch22\n0000"
17:59:40.271553 pktline.go:233: packet: < 000e "unpack ok\n"
17:59:40.271562 pktline.go:233: packet: < 001b "ok refs/heads/branch22\n"
17:59:40.271567 pktline.go:132: packet: < 0000

git cli (github):

18:08:07.494227 git.c:476               trace: built-in: git push origin branch22
18:08:07.496115 run-command.c:667       trace: run_command: unset GIT_PREFIX; ssh git@github.com 'git-receive-pack '\''aymanbagabas/dummy.git'\'''
18:08:07.496195 run-command.c:759       trace: start_command: /usr/bin/ssh git@github.com 'git-receive-pack '\''aymanbagabas/dummy.git'\'''
18:08:08.026220 pkt-line.c:85           packet:         push< 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\0report-status report-status-v2 delete-refs side-band-64k ofs-delta atomic object-format=sha1 quiet agent=github/spokes-receive-pack-acac8763c60f636c44baaf5c3887895cf5f55c30 session-id=8eebfe0e641309161dcb35f250e6fc41 push-options
18:08:08.026343 pkt-line.c:85           packet:         push< b0dc1ca52c9960e585b5807ec4e876b8768014ba refs/heads/master
18:08:08.026365 pkt-line.c:85           packet:         push< 1e1bd0038f6cb55321f83f19eb84139daa5c2027 refs/heads/newbranch
18:08:08.026378 pkt-line.c:85           packet:         push< 0000
18:08:08.026748 pkt-line.c:85           packet:         push> 0000000000000000000000000000000000000000 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/branch22\0 report-status-v2 side-band-64k object-format=sha1 agent=git/2.49.0-Darwin
18:08:08.026782 pkt-line.c:85           packet:         push> 0000
18:08:08.026877 run-command.c:667       trace: run_command: git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
18:08:08.026935 run-command.c:759       trace: start_command: /nix/store/ir2bhb7n3ngh0dg7p6jbhm1wa4d8gkb7-git-2.49.0/libexec/git-core/git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
18:08:08.038722 git.c:476               trace: built-in: git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
18:08:08.056901 pkt-line.c:85           packet:     sideband< \2\0
18:08:08.200541 pkt-line.c:85           packet:     sideband< \1000eunpack ok
18:08:08.200587 pkt-line.c:85           packet:     sideband< \1001bok refs/heads/branch22
18:08:08.200617 pkt-line.c:85           packet:     sideband< \2Create a pull request for 'branch22' on GitHub by visiting:     https://github.com/aymanbagabas/dummy/pull/new/branch22
remote:
remote: Create a pull request for 'branch22' on GitHub by visiting:
18:08:08.200623 pkt-line.c:85           packet:         push< unpack ok
remote:      https://github.com/aymanbagabas/dummy/pull/new/branch22
remote:
18:08:08.200656 pkt-line.c:85           packet:     sideband< \10000
18:08:08.200660 pkt-line.c:85           packet:         push< ok refs/heads/branch22
18:08:08.200665 pkt-line.c:85           packet:     sideband< 0000
18:08:08.200681 pkt-line.c:85           packet:         push< 0000
To github.com:aymanbagabas/dummy.git
 * [new branch]                branch22 -> branch22

Go-git cli (github):

18:09:06.603014 pktline.go:233: packet: < 0123 "74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\x00report-status report-status-v2 delete-refs side-band-64k ofs-delta atomic object-format=sha1 quiet agent=github/spokes-receive-pack-acac8763c60f636c44baaf5c3887895cf5f55c30 session-id=cda4c4be4a4bd8bd936c09093caa89d7 push-options\n"
18:09:06.603189 pktline.go:233: packet: < 0123 "74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/b123\x00report-status report-status-v2 delete-refs side-band-64k ofs-delta atomic object-format=sha1 quiet agent=github/spokes-receive-pack-acac8763c60f636c44baaf5c3887895cf5f55c30 session-id=cda4c4be4a4bd8bd936c09093caa89d7 push-options\n"
18:09:06.603293 pktline.go:233: packet: < 003f "b0dc1ca52c9960e585b5807ec4e876b8768014ba refs/heads/master\n"
18:09:06.603311 pktline.go:233: packet: < 0042 "1e1bd0038f6cb55321f83f19eb84139daa5c2027 refs/heads/newbranch\n"
18:09:06.603320 pktline.go:132: packet: < 0000
18:09:06.608082 pktline.go:233: packet: > 009d "0000000000000000000000000000000000000000 74b494bccf820409c7f9405b3a6e3c0eb490202b refs/heads/branch22\x00report-status side-band-64k agent=git/2.49.0-Darwin"
18:09:06.608193 pktline.go:72: packet: > 0000
18:09:06.629675 pktline.go:233: packet: < 0006 "\x02\x00"
18:09:06.738222 pktline.go:233: packet: < 0013 "\x01000eunpack ok\n"
18:09:06.738266 pktline.go:233: packet: < 000e "unpack ok\n"
18:09:06.738325 pktline.go:233: packet: < 0020 "\x01001bok refs/heads/branch22\n"
18:09:06.738349 pktline.go:233: packet: < 001b "ok refs/heads/branch22\n"
18:09:06.738382 pktline.go:233: packet: < 0080 "\x02\nCreate a pull request for 'branch22' on GitHub by visiting:\n     https://github.com/aymanbagabas/dummy/pull/new/branch22\n\n"

Create a pull request for 'branch22' on GitHub by visiting:
     https://github.com/aymanbagabas/dummy/pull/new/branch22

18:09:06.738407 pktline.go:233: packet: < 0009 "\x010000"
18:09:06.738415 pktline.go:132: packet: < 0000

UrosSimovic · 2025-07-10T11:57:52Z

You can notice that GitLab does not send their post-receive server updates when the client is Go-git.

That is not true. It sends it just fine, but go-git stops reading it after first flush.
I tested it with git over https and mitm proxy in linked issue.

If you check both of your git cli debug outputs, the diff between them is that gitlab (as well as vanilla git by my findings)
sends additional flush between status and post receive message:

...
18:03:52.774640 pkt-line.c:85           packet:         push< 0000
...

And this PR tries to fix exactly that - don't stop reading the output after first flush.

feat: allow multiple flushes when decoding report status

8c8be7c

UrosSimovic commented Jul 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: allow multiple flushes when decoding report status #1597

feat: allow multiple flushes when decoding report status #1597

Uh oh!

UrosSimovic commented Jul 1, 2025

Uh oh!

UrosSimovic Jul 1, 2025

Uh oh!

UrosSimovic commented Jul 1, 2025

Uh oh!

aymanbagabas commented Jul 7, 2025

Uh oh!

UrosSimovic commented Jul 7, 2025 •

edited

Loading

Uh oh!

aymanbagabas commented Jul 7, 2025 •

edited

Loading

Uh oh!

UrosSimovic commented Jul 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

feat: allow multiple flushes when decoding report status #1597

Are you sure you want to change the base?

feat: allow multiple flushes when decoding report status #1597

Uh oh!

Conversation

UrosSimovic commented Jul 1, 2025

Uh oh!

UrosSimovic Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

UrosSimovic commented Jul 1, 2025

Uh oh!

aymanbagabas commented Jul 7, 2025

Uh oh!

UrosSimovic commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aymanbagabas commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UrosSimovic commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

UrosSimovic commented Jul 7, 2025 •

edited

Loading

aymanbagabas commented Jul 7, 2025 •

edited

Loading

UrosSimovic commented Jul 10, 2025 •

edited

Loading