git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git push --atomic and HTTP(S) vs SSH
@ 2019-04-09  6:11 Bryan Turner
  2019-04-09  9:58 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Bryan Turner @ 2019-04-09  6:11 UTC (permalink / raw)
  To: Git Users

A question came up on Stack Overflow[1], and then again through our
support channels, about "git push --atomic" and a behavior mismatch
between HTTP(S) and SSH. I'm easily able to reproduce the behavior,
but I don't see anything Bitbucket Server-related about how this works
(and I get exactly the same mismatched behavior against GitHub; but
that's not really a surprise given what I see in the wire protocol.

First, a setup: 2 commits, with the second commit then amended to make
a 3rd commit.

git init foo
cd foo
echo 'Hello, World' > file.txt
git add file.txt
git commit -m "Initial commit"
echo 'Goodbye, World' > file.txt
git commit -am "Second commit"
git tag original-message
echo 'Goodbye, Cruel World' > file.txt
git commit -a --amend --no-edit
git tag amended-message

Now, given an HTTP remote: (Assume "atomic-pushes" is an empty,
just-created repository)

git remote add http https://github.com/bturner/atomic-pushes.git
git push http original-message:refs/heads/master
git push --atomic http amended-message:refs/heads/master
amended-message:refs/heads/branch-1

So we push the original message commit to master first, and then we
try to push (without --force) the amended commit to both master and a
new branch. That'll produce this:

Total 0 (delta 0), reused 0 (delta 0)
remote:
remote: Create a pull request for 'branch-1' on GitHub by visiting:
remote:      https://github.com/bturner/atomic-pushes/pull/new/branch-1
remote:
To https://github.com/bturner/atomic-pushes.git
 * [new branch]      HEAD -> branch-1
 ! [rejected]        HEAD -> master (non-fast-forward)
error: failed to push some refs to
'https://github.com/bturner/atomic-pushes.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

Now let's try it with SSH: (Using a different branch because the HTTP
push created branch-1)

git remote add ssh git@github.com:bturner/atomic-pushes.git
git push --atomic ssh amended-message:refs/heads/master
amended-message:refs/heads/branch-2

Now we see:

error: atomic push failed for ref refs/heads/master. status: 2

To git@github.com:bturner/atomic-pushes.git
 ! [rejected]        HEAD -> master (non-fast-forward)
error: failed to push some refs to 'git@github.com:bturner/atomic-pushes.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

No mention of branch-2 at all, but if I do a "git ls-remote ssh" I see:

fe86a3eae65e18787040499c17a567096159b9ce HEAD
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-1
fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master

So branch-2 didn't get created.

Looking at the wire protocol with GIT_TRACE_PACKET, I see this
conversation for HTTP: (At this point I've done several tests, which
have created various branches, so I'm now trying to push master and
branch-4)

22:16:06.562939 pkt-line.c:46           packet:          git< #
service=git-receive-pack
22:16:06.562990 pkt-line.c:46           packet:          git< 0000
22:16:06.562994 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
atomic ofs-delta agent=git/github-g4f6c801f9475
22:16:06.563013 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
22:16:06.563016 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
22:16:06.563019 pkt-line.c:46           packet:          git<
fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
22:16:06.563024 pkt-line.c:46           packet:          git< 0000
22:16:06.563329 pkt-line.c:46           packet:          git>
HEAD:refs/heads/branch-4
22:16:06.563346 pkt-line.c:46           packet:          git> 0000
22:16:06.563357 run-command.c:347       trace: run_command:
'send-pack' '--stateless-rpc' '--helper-status' '--thin' '--progress'
'https://github.com/bturner/atomic-pushes.git/' '--stdin'
22:16:06.563765 exec_cmd.c:129          trace: exec: 'git' 'send-pack'
'--stateless-rpc' '--helper-status' '--thin' '--progress'
'https://github.com/bturner/atomic-pushes.git/' '--stdin'
22:16:06.564691 git.c:348               trace: built-in: git
'send-pack' '--stateless-rpc' '--helper-status' '--thin' '--progress'
'https://github.com/bturner/atomic-pushes.git/' '--stdin'
22:16:06.564788 pkt-line.c:46           packet:          git<
HEAD:refs/heads/branch-4
22:16:06.564793 pkt-line.c:46           packet:          git< 0000
22:16:06.564797 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
atomic ofs-delta agent=git/github-g4f6c801f9475
22:16:06.564805 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
22:16:06.564826 pkt-line.c:46           packet:          git<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
22:16:06.564830 pkt-line.c:46           packet:          git<
fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
22:16:06.564834 pkt-line.c:46           packet:          git< 0000
22:16:06.564970 pkt-line.c:46           packet:          git>
0000000000000000000000000000000000000000
6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4\0
report-status side-band-64k agent=git/2.4.0
22:16:06.564989 pkt-line.c:46           packet:          git> 0000
22:16:06.565027 pkt-line.c:46           packet:          git<
00960000000000000000000000000000000000000000
6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4\0
report-status side-band-64k agent=git/2.4.00000

Based on that, I see that the server advertises the existing refs, and
"atomic" support, and then the client _only_ sends branch-4; it
doesn't send anything for master at all. That implies the
non-fast-forward rejection for master is actually being done locally,
not on the server. Only one ref change gets sent to the server, which
applies without issue.

Looking at SSH shows very different output: (branch-4 now exists
because of the HTTP test above, so now I'm pushing master and
branch-5)

22:56:08.609608 pkt-line.c:46           packet:         push<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
atomic ofs-delta agent=git/github-g4f6c801f9475
22:56:08.609774 pkt-line.c:46           packet:         push<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
22:56:08.609798 pkt-line.c:46           packet:         push<
1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
22:56:08.609801 pkt-line.c:46           packet:         push<
6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4
22:56:08.609825 pkt-line.c:46           packet:         push<
fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
22:56:08.609831 pkt-line.c:46           packet:         push< 0000

There are no "push>" lines at all. The server sends its ref
advertisement, and then it seems like the non-fast-forward rejection
for master happens locally, similar to HTTPS, but then, unlike HTTPS,
the SSH push is simply aborted without ever sending anything to the
server at all.

I've verified this with Git 2.4.0 as a client (which appears to be
where "git push --atomic" was first introduced), and with Git 2.20.1
as a client, to ensure there wasn't a regression somewhere, and the
behavior is identical for both versions. (I've also verified it with
both Linux and Windows clients, just for extra paranoia.)

The result seems unexpected from a user perspective, but looking at
the code it seems like it's working as implemented. "remote-curl.c"
doesn't include "--atomic" when it runs "git send-pack", so it looks
like HTTP(S) pushes simply ignore "git push --atomic" entirely.

Am I misunderstanding how this is supposed to work? If not, should the
documentation be updated to clarify that HTTP(S) remotes don't support
"--atomic"?

Best regards,
Bryan Turner

[1] https://stackoverflow.com/questions/37531663/git-push-atomic-not-failing

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: git push --atomic and HTTP(S) vs SSH
  2019-04-09  6:11 git push --atomic and HTTP(S) vs SSH Bryan Turner
@ 2019-04-09  9:58 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2019-04-09  9:58 UTC (permalink / raw)
  To: Bryan Turner; +Cc: Git Users

Bryan Turner <bturner@atlassian.com> writes:

> A question came up on Stack Overflow[1], and then again through our
> support channels, about "git push --atomic" and a behavior mismatch
> between HTTP(S) and SSH. I'm easily able to reproduce the behavior,
> but I don't see anything Bitbucket Server-related about how this works
> (and I get exactly the same mismatched behavior against GitHub; but
> that's not really a surprise given what I see in the wire protocol.
>
> First, a setup: 2 commits, with the second commit then amended to make
> a 3rd commit.
>
> git init foo
> cd foo
> echo 'Hello, World' > file.txt
> git add file.txt
> git commit -m "Initial commit"
> echo 'Goodbye, World' > file.txt
> git commit -am "Second commit"
> git tag original-message
> echo 'Goodbye, Cruel World' > file.txt
> git commit -a --amend --no-edit
> git tag amended-message
>
> Now, given an HTTP remote: (Assume "atomic-pushes" is an empty,
> just-created repository)
>
> git remote add http https://github.com/bturner/atomic-pushes.git
> git push http original-message:refs/heads/master
> git push --atomic http amended-message:refs/heads/master
> amended-message:refs/heads/branch-1
>
> So we push the original message commit to master first, and then we
> try to push (without --force) the amended commit to both master and a
> new branch. That'll produce this:
>
> Total 0 (delta 0), reused 0 (delta 0)
> remote:
> remote: Create a pull request for 'branch-1' on GitHub by visiting:
> remote:      https://github.com/bturner/atomic-pushes/pull/new/branch-1
> remote:
> To https://github.com/bturner/atomic-pushes.git
>  * [new branch]      HEAD -> branch-1
>  ! [rejected]        HEAD -> master (non-fast-forward)
> error: failed to push some refs to
> 'https://github.com/bturner/atomic-pushes.git'
> hint: Updates were rejected because the tip of your current branch is behind
> hint: its remote counterpart. Integrate the remote changes (e.g.
> hint: 'git pull ...') before pushing again.
> hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>
> Now let's try it with SSH: (Using a different branch because the HTTP
> push created branch-1)
>
> git remote add ssh git@github.com:bturner/atomic-pushes.git
> git push --atomic ssh amended-message:refs/heads/master
> amended-message:refs/heads/branch-2
>
> Now we see:
>
> error: atomic push failed for ref refs/heads/master. status: 2
>
> To git@github.com:bturner/atomic-pushes.git
>  ! [rejected]        HEAD -> master (non-fast-forward)
> error: failed to push some refs to 'git@github.com:bturner/atomic-pushes.git'
> hint: Updates were rejected because the tip of your current branch is behind
> hint: its remote counterpart. Integrate the remote changes (e.g.
> hint: 'git pull ...') before pushing again.
> hint: See the 'Note about fast-forwards' in 'git push --help' for details.
>
> No mention of branch-2 at all, but if I do a "git ls-remote ssh" I see:
>
> fe86a3eae65e18787040499c17a567096159b9ce HEAD
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-1
> fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
>
> So branch-2 didn't get created.
>
> Looking at the wire protocol with GIT_TRACE_PACKET, I see this
> conversation for HTTP: (At this point I've done several tests, which
> have created various branches, so I'm now trying to push master and
> branch-4)
>
> 22:16:06.562939 pkt-line.c:46           packet:          git< #
> service=git-receive-pack
> 22:16:06.562990 pkt-line.c:46           packet:          git< 0000
> 22:16:06.562994 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
> refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
> atomic ofs-delta agent=git/github-g4f6c801f9475
> 22:16:06.563013 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
> 22:16:06.563016 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
> 22:16:06.563019 pkt-line.c:46           packet:          git<
> fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
> 22:16:06.563024 pkt-line.c:46           packet:          git< 0000
> 22:16:06.563329 pkt-line.c:46           packet:          git>
> HEAD:refs/heads/branch-4
> 22:16:06.563346 pkt-line.c:46           packet:          git> 0000
> 22:16:06.563357 run-command.c:347       trace: run_command:
> 'send-pack' '--stateless-rpc' '--helper-status' '--thin' '--progress'
> 'https://github.com/bturner/atomic-pushes.git/' '--stdin'
> 22:16:06.563765 exec_cmd.c:129          trace: exec: 'git' 'send-pack'
> '--stateless-rpc' '--helper-status' '--thin' '--progress'
> 'https://github.com/bturner/atomic-pushes.git/' '--stdin'
> 22:16:06.564691 git.c:348               trace: built-in: git
> 'send-pack' '--stateless-rpc' '--helper-status' '--thin' '--progress'
> 'https://github.com/bturner/atomic-pushes.git/' '--stdin'
> 22:16:06.564788 pkt-line.c:46           packet:          git<
> HEAD:refs/heads/branch-4
> 22:16:06.564793 pkt-line.c:46           packet:          git< 0000
> 22:16:06.564797 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
> refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
> atomic ofs-delta agent=git/github-g4f6c801f9475
> 22:16:06.564805 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
> 22:16:06.564826 pkt-line.c:46           packet:          git<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
> 22:16:06.564830 pkt-line.c:46           packet:          git<
> fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
> 22:16:06.564834 pkt-line.c:46           packet:          git< 0000
> 22:16:06.564970 pkt-line.c:46           packet:          git>
> 0000000000000000000000000000000000000000
> 6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4\0
> report-status side-band-64k agent=git/2.4.0
> 22:16:06.564989 pkt-line.c:46           packet:          git> 0000
> 22:16:06.565027 pkt-line.c:46           packet:          git<
> 00960000000000000000000000000000000000000000
> 6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4\0
> report-status side-band-64k agent=git/2.4.00000
>
> Based on that, I see that the server advertises the existing refs, and
> "atomic" support, and then the client _only_ sends branch-4; it
> doesn't send anything for master at all. That implies the
> non-fast-forward rejection for master is actually being done locally,
> not on the server. Only one ref change gets sent to the server, which
> applies without issue.
>
> Looking at SSH shows very different output: (branch-4 now exists
> because of the HTTP test above, so now I'm pushing master and
> branch-5)
>
> 22:56:08.609608 pkt-line.c:46           packet:         push<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319
> refs/heads/branch-1\0report-status delete-refs side-band-64k quiet
> atomic ofs-delta agent=git/github-g4f6c801f9475
> 22:56:08.609774 pkt-line.c:46           packet:         push<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-2
> 22:56:08.609798 pkt-line.c:46           packet:         push<
> 1b9c21b7aeb6ad03957cc8a023b2406d3ccee319 refs/heads/branch-3
> 22:56:08.609801 pkt-line.c:46           packet:         push<
> 6925c65344e87c65e5cd2b56d392cd06ef96ca71 refs/heads/branch-4
> 22:56:08.609825 pkt-line.c:46           packet:         push<
> fe86a3eae65e18787040499c17a567096159b9ce refs/heads/master
> 22:56:08.609831 pkt-line.c:46           packet:         push< 0000
>
> There are no "push>" lines at all. The server sends its ref
> advertisement, and then it seems like the non-fast-forward rejection
> for master happens locally, similar to HTTPS, but then, unlike HTTPS,
> the SSH push is simply aborted without ever sending anything to the
> server at all.

That sounds correct.  Behaviour you saw on the smart-http side seems
utterly broken.

FWIW, I think the client-side rejection is merely an optimization,
and if you changed the ref at the receiving end between the time the
receiver advertised its refs and the sender finished the push, the
receiver should notice non-fast-forward-ness and reject the whole
operation (this is probably very hard to write an automated test
for).

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-04-09  9:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09  6:11 git push --atomic and HTTP(S) vs SSH Bryan Turner
2019-04-09  9:58 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).