HomePhabricator

When pushing changes to staging, also push the base commit

Description

When pushing changes to staging, also push the base commit

Summary:
Fixes T10509. Pushing changes to staging can be inefficient. What happens, roughly, is:

  • Master is at commit "W" -- "W" is the most recent published commit in the main repository.
  • The local working copy has one change on top of that, "X", so its history is commits "A, B, C, D, E, F, ..., U, V, W, X".
  • The remote has some other previous changes that I or other users have made, maybe like "A, B, C, ..., S, T, U, Y" and "A, B, C, ..., T, U, V, Z", from previous pushes to staging areas.
  • "X", "Y" and "Z" will never actually make it to master, because they'll be squash-merged/amended by arc land.

So the local says "I want to push 'X'", and the remote says "I know about 'Y' and 'Z', are those in the history of 'W'? You only need to send me new stuff if they are".

But they aren't, so the local says "nope, so here's the whole history for you". This is slow and sends a ton of data that the remote already has over the network.

In theory, Git could use a slightly different algorithm to tell the local about more commits, but this is hard, rarely useful, and not the kind of thing I'd be excited about changing if I was the Git upstream.

Instead, when pushing "X", also push "W", to trick Git into telling future clients about it.

Now, the remote should say "I know about 'W', 'Y' and 'Z'", and the local will say "oh, great, 'W' is in history, here's just the changes since then".

Also, fail arc diff if the push to staging fails, and tell users to use --skip-staging. This code has been in production for a while and doesn't seem to have any issues, and a failed push to staging prevents builds, lands, etc.

Test Plan:

  • Ran arc diff, saw two changes push.
  • Ran arc diff --base arc:empty, saw only one change push.
  • Ran arc diff with an intentionally broken staging area, saw an error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10509

Differential Revision: https://secure.phabricator.com/D15424