revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-29 00:40:57 +01:00
Code Issues Releases Wiki Activity

When pushing changes to staging, also push the base commit

Browse source 
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
This commit is contained in:
epriestley 2016-03-07 06:17:43 -08:00
parent 604ceb4fe5
commit 4a1160e0c3
2 changed files with 30 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/upload/ArcanistFileUploader.php
View file

@ -26,7 +26,7 @@
final class ArcanistFileUploader extends Phobject {
private $conduit;
private $files;
private $files = array();
/* -( Configuring the Uploader )------------------------------------------- */

38
src/workflow/ArcanistDiffWorkflow.php
View file

@ -2713,7 +2713,9 @@ EOTEXT
$commit = $api->getHeadCommit();
$prefix = idx($staging, 'prefix', 'phabricator');
$tag = $prefix.'/diff/'.$id;
$base_tag = $prefix.'/base/'.$id;
$diff_tag = $prefix.'/diff/'.$id;
$this->writeOkay(
pht('PUSH STAGING'),
@ -2723,23 +2725,41 @@ EOTEXT
if (version_compare($api->getGitVersion(), '1.8.2', '>=')) {
$push_flags[] = '--no-verify';
}
$refs = array();
// If the base commit is a real commit, we're going to push it. We don't
// use this, but pushing it to a ref reduces the amount of redundant work
// that Git does on later pushes by helping it figure out that the remote
// already has most of the history. See T10509.
// In the future, we could avoid this push if the staging area is the same
// as the main repository, or if the staging area is a virtual repository.
// In these cases, the staging area should automatically have up-to-date
// refs.
$base_commit = $api->getSourceControlBaseRevision();
if ($base_commit !== ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) {
$refs[] = "{$base_commit}:refs/tags/{$base_tag}";
}
// We're always going to push the change itself.
$refs[] = "{$commit}:refs/tags/{$diff_tag}";
$err = phutil_passthru(
'git push %Ls -- %s %s:refs/tags/%s',
'git push %Ls -- %s %Ls',
$push_flags,
$staging_uri,
$commit,
$tag);
$refs);
if ($err) {
$this->writeWarn(
pht('STAGING FAILED'),
pht('Unable to push changes to the staging area.'));
} else {
$this->writeOkay(
pht('STAGING PUSHED'),
throw new ArcanistUsageException(
pht(
'Pushed a copy of the changes to tag "%s" in the staging area.',
$tag));
'Failed to push changes to staging area. Correct the issue, or '.
'use --skip-staging to skip this step.'));
}
}