Bryan Turner <bturner@xxxxxxxxxxxxx> 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@xxxxxxxxxx: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@xxxxxxxxxx:bturner/atomic-pushes.git > ! [rejected] HEAD -> master (non-fast-forward) > error: failed to push some refs to 'git@xxxxxxxxxx: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).
