2011-01-10 00:22:25 +01:00
|
|
|
<?php
|
|
|
|
|
2011-02-19 20:36:08 +01:00
|
|
|
/**
|
|
|
|
* Sends changes from your working copy to Differential for code review.
|
|
|
|
*
|
2012-01-05 20:26:17 +01:00
|
|
|
* @task lintunit Lint and Unit Tests
|
|
|
|
* @task message Commit and Update Messages
|
|
|
|
* @task diffspec Diff Specification
|
|
|
|
* @task diffprop Diff Properties
|
2011-02-19 20:36:08 +01:00
|
|
|
*/
|
2014-07-21 23:49:15 +02:00
|
|
|
final class ArcanistDiffWorkflow extends ArcanistWorkflow {
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
private $console;
|
2011-01-10 00:22:25 +01:00
|
|
|
private $hasWarnedExternals = false;
|
2011-01-12 05:21:17 +01:00
|
|
|
private $unresolvedLint;
|
2012-10-22 23:46:31 +02:00
|
|
|
private $excuses = array('lint' => null, 'unit' => null);
|
2012-01-31 21:07:19 +01:00
|
|
|
private $testResults;
|
2011-01-22 01:17:30 +01:00
|
|
|
private $diffID;
|
2012-02-23 18:18:49 +01:00
|
|
|
private $revisionID;
|
2012-07-03 00:53:22 +02:00
|
|
|
private $postponedLinters;
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
private $haveUncommittedChanges = false;
|
2012-10-13 00:01:19 +02:00
|
|
|
private $diffPropertyFutures = array();
|
2012-10-16 20:51:15 +02:00
|
|
|
private $commitMessageFromRevision;
|
2011-01-10 00:22:25 +01:00
|
|
|
|
Make Arcanist workflow names explicit
Summary:
Currently, adding a new workflow requires you to override ArcanistConfiguration, which is messy. Instead, just load everything that extends ArcanistBaseWorkflow.
Remove all the rules tying workflow names to class names through arcane incantations.
This has a very small performance cost in that we need to load every Workflow class every time now, but we don't hit __init__ and such anymore and it was pretty negligible on my machine (98ms vs 104ms or something).
Test Plan: Ran "arc help", "arc which", "arc diff", etc.
Reviewers: edward, vrana, btrahan
Reviewed By: edward
CC: aran, zeeg
Differential Revision: https://secure.phabricator.com/D3691
2012-10-17 17:35:03 +02:00
|
|
|
public function getWorkflowName() {
|
|
|
|
return 'diff';
|
|
|
|
}
|
|
|
|
|
2012-03-05 19:02:37 +01:00
|
|
|
public function getCommandSynopses() {
|
2011-01-10 00:22:25 +01:00
|
|
|
return phutil_console_format(<<<EOTEXT
|
|
|
|
**diff** [__paths__] (svn)
|
2011-12-03 00:50:36 +01:00
|
|
|
**diff** [__commit__] (git, hg)
|
2012-03-05 19:02:37 +01:00
|
|
|
EOTEXT
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getCommandHelp() {
|
|
|
|
return phutil_console_format(<<<EOTEXT
|
2011-12-03 00:50:36 +01:00
|
|
|
Supports: git, svn, hg
|
2011-01-10 00:22:25 +01:00
|
|
|
Generate a Differential diff or revision from local changes.
|
|
|
|
|
2013-02-08 16:09:30 +01:00
|
|
|
Under git and mercurial, you can specify a commit (like __HEAD^^^__
|
|
|
|
or __master__) and Differential will generate a diff against the
|
|
|
|
merge base of that commit and your current working directory parent.
|
2011-01-10 00:22:25 +01:00
|
|
|
|
|
|
|
Under svn, you can choose to include only some of the modified files
|
|
|
|
in the working copy in the diff by specifying their paths. If you
|
|
|
|
omit paths, all changes are included in the diff.
|
|
|
|
EOTEXT
|
|
|
|
);
|
|
|
|
}
|
2012-06-27 20:21:19 +02:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
public function requiresWorkingCopy() {
|
2012-01-05 22:36:24 +01:00
|
|
|
return !$this->isRawDiffSource();
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
public function requiresConduit() {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function requiresAuthentication() {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function requiresRepositoryAPI() {
|
2012-03-05 19:08:50 +01:00
|
|
|
if (!$this->isRawDiffSource()) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->getArgument('use-commit-message')) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
return false;
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
2011-01-30 02:18:32 +01:00
|
|
|
|
2011-01-22 01:17:30 +01:00
|
|
|
public function getDiffID() {
|
|
|
|
return $this->diffID;
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
|
|
|
|
public function getArguments() {
|
2012-09-18 23:14:12 +02:00
|
|
|
$arguments = array(
|
2011-01-10 00:22:25 +01:00
|
|
|
'message' => array(
|
|
|
|
'short' => 'm',
|
|
|
|
'param' => 'message',
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'When updating a revision, use the specified message instead of '.
|
|
|
|
'prompting.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
2011-12-03 00:50:36 +01:00
|
|
|
'message-file' => array(
|
|
|
|
'short' => 'F',
|
|
|
|
'param' => 'file',
|
|
|
|
'paramtype' => 'file',
|
|
|
|
'help' => 'When creating a revision, read revision information '.
|
|
|
|
'from this file.',
|
|
|
|
),
|
2012-01-13 04:03:11 +01:00
|
|
|
'use-commit-message' => array(
|
|
|
|
'supports' => array(
|
|
|
|
'git',
|
|
|
|
// TODO: Support mercurial.
|
|
|
|
),
|
|
|
|
'short' => 'C',
|
|
|
|
'param' => 'commit',
|
|
|
|
'help' => 'Read revision information from a specific commit.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'only' => null,
|
|
|
|
'preview' => null,
|
|
|
|
'update' => null,
|
|
|
|
),
|
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
'edit' => array(
|
|
|
|
'supports' => array(
|
|
|
|
'git',
|
2013-02-09 20:24:54 +01:00
|
|
|
'hg',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
|
|
|
'nosupport' => array(
|
|
|
|
'svn' => 'Edit revisions via the web interface when using SVN.',
|
|
|
|
),
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'When updating a revision under git, edit revision information '.
|
|
|
|
'before updating.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
2012-01-05 22:36:24 +01:00
|
|
|
'raw' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Read diff from stdin, not from the working copy. This disables '.
|
|
|
|
'many Arcanist/Phabricator features which depend on having access '.
|
|
|
|
'to the working copy.',
|
2012-01-05 22:36:24 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'less-context' => null,
|
|
|
|
'apply-patches' => '--raw disables lint.',
|
|
|
|
'never-apply-patches' => '--raw disables lint.',
|
2012-11-19 22:20:44 +01:00
|
|
|
'advice' => '--raw disables lint.',
|
2012-01-05 22:36:24 +01:00
|
|
|
'lintall' => '--raw disables lint.',
|
|
|
|
|
|
|
|
'create' => '--raw and --create both need stdin. '.
|
|
|
|
'Use --raw-command.',
|
|
|
|
'edit' => '--raw and --edit both need stdin. '.
|
|
|
|
'Use --raw-command.',
|
|
|
|
'raw-command' => null,
|
|
|
|
),
|
|
|
|
),
|
|
|
|
'raw-command' => array(
|
|
|
|
'param' => 'command',
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Generate diff by executing a specified command, not from the '.
|
|
|
|
'working copy. This disables many Arcanist/Phabricator features '.
|
|
|
|
'which depend on having access to the working copy.',
|
2012-01-05 22:36:24 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'less-context' => null,
|
|
|
|
'apply-patches' => '--raw-command disables lint.',
|
|
|
|
'never-apply-patches' => '--raw-command disables lint.',
|
2012-11-19 22:20:44 +01:00
|
|
|
'advice' => '--raw-command disables lint.',
|
2012-01-05 22:36:24 +01:00
|
|
|
'lintall' => '--raw-command disables lint.',
|
|
|
|
),
|
|
|
|
),
|
2011-12-03 00:50:36 +01:00
|
|
|
'create' => array(
|
2014-05-23 22:53:05 +02:00
|
|
|
'help' => 'Always create a new revision.',
|
2011-12-03 00:50:36 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'edit' => '--create can not be used with --edit.',
|
2012-01-05 20:26:17 +01:00
|
|
|
'only' => '--create can not be used with --only.',
|
|
|
|
'preview' => '--create can not be used with --preview.',
|
2012-01-09 18:33:55 +01:00
|
|
|
'update' => '--create can not be used with --update.',
|
2011-12-03 00:50:36 +01:00
|
|
|
),
|
|
|
|
),
|
2012-01-09 18:33:55 +01:00
|
|
|
'update' => array(
|
|
|
|
'param' => 'revision_id',
|
2014-05-23 22:53:05 +02:00
|
|
|
'help' => 'Always update a specific revision.',
|
2012-01-09 18:33:55 +01:00
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
'nounit' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Do not run unit tests.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
|
|
|
'nolint' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Do not run lint.',
|
2011-01-10 00:22:25 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'lintall' => '--nolint suppresses lint.',
|
2012-11-19 22:20:44 +01:00
|
|
|
'advice' => '--nolint suppresses lint.',
|
2011-01-10 23:03:12 +01:00
|
|
|
'apply-patches' => '--nolint suppresses lint.',
|
|
|
|
'never-apply-patches' => '--nolint suppresses lint.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
|
|
|
),
|
|
|
|
'only' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Only generate a diff, without running lint, unit tests, or other '.
|
|
|
|
'auxiliary steps. See also --preview.',
|
2011-01-10 00:22:25 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'preview' => null,
|
|
|
|
'message' => '--only does not affect revisions.',
|
|
|
|
'edit' => '--only does not affect revisions.',
|
|
|
|
'lintall' => '--only suppresses lint.',
|
2012-11-19 22:20:44 +01:00
|
|
|
'advice' => '--only suppresses lint.',
|
2011-01-10 23:03:12 +01:00
|
|
|
'apply-patches' => '--only suppresses lint.',
|
|
|
|
'never-apply-patches' => '--only suppresses lint.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
|
|
|
),
|
|
|
|
'preview' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Instead of creating or updating a revision, only create a diff, '.
|
|
|
|
'which you may later attach to a revision. This still runs lint '.
|
|
|
|
'unit tests. See also --only.',
|
2011-01-10 00:22:25 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'only' => null,
|
|
|
|
'edit' => '--preview does affect revisions.',
|
|
|
|
'message' => '--preview does not update any revision.',
|
|
|
|
),
|
|
|
|
),
|
2012-12-05 23:04:10 +01:00
|
|
|
'plan-changes' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Create or update a revision without requesting a code review.',
|
2012-12-05 23:04:10 +01:00
|
|
|
'conflicts' => array(
|
|
|
|
'only' => '--only does not affect revisions.',
|
|
|
|
'preview' => '--preview does not affect revisions.',
|
|
|
|
),
|
|
|
|
),
|
2011-12-01 17:56:03 +01:00
|
|
|
'encoding' => array(
|
|
|
|
'param' => 'encoding',
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Attempt to convert non UTF-8 hunks into specified encoding.',
|
2011-12-01 17:56:03 +01:00
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
'allow-untracked' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Skip checks for untracked files in the working copy.',
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
2012-03-01 01:30:17 +01:00
|
|
|
'excuse' => array(
|
|
|
|
'param' => 'excuse',
|
|
|
|
'help' => 'Provide a prepared in advance excuse for any lints/tests'.
|
|
|
|
' shall they fail.',
|
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
'less-context' => array(
|
|
|
|
'help' =>
|
|
|
|
"Normally, files are diffed with full context: the entire file is ".
|
|
|
|
"sent to Differential so reviewers can 'show more' and see it. If ".
|
|
|
|
"you are making changes to very large files with tens of thousands ".
|
|
|
|
"of lines, this may not work well. With this flag, a diff will ".
|
|
|
|
"be created that has only a few lines of context.",
|
|
|
|
),
|
|
|
|
'lintall' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Raise all lint warnings, not just those on lines you changed.',
|
2011-02-02 05:32:10 +01:00
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
),
|
2012-11-19 22:20:44 +01:00
|
|
|
'advice' => array(
|
|
|
|
'help' =>
|
2014-05-23 22:53:05 +02:00
|
|
|
'Require excuse for lint advice in addition to lint warnings and '.
|
|
|
|
'errors.',
|
2012-11-19 22:20:44 +01:00
|
|
|
),
|
2012-12-04 01:52:58 +01:00
|
|
|
'only-new' => array(
|
|
|
|
'param' => 'bool',
|
|
|
|
'help' =>
|
|
|
|
'Display only lint messages not present in the original code.',
|
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
|
|
|
),
|
2011-01-10 23:03:12 +01:00
|
|
|
'apply-patches' => array(
|
|
|
|
'help' =>
|
|
|
|
'Apply patches suggested by lint to the working copy without '.
|
|
|
|
'prompting.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'never-apply-patches' => true,
|
|
|
|
),
|
2011-02-02 05:32:10 +01:00
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
2011-01-10 23:03:12 +01:00
|
|
|
),
|
|
|
|
'never-apply-patches' => array(
|
|
|
|
'help' => 'Never apply patches suggested by lint.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'apply-patches' => true,
|
|
|
|
),
|
2011-02-02 05:32:10 +01:00
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
2011-01-10 23:03:12 +01:00
|
|
|
),
|
2012-04-06 21:23:19 +02:00
|
|
|
'amend-all' => array(
|
|
|
|
'help' =>
|
|
|
|
'When linting git repositories, amend HEAD with all patches '.
|
|
|
|
'suggested by lint without prompting.',
|
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
|
|
|
),
|
|
|
|
'amend-autofixes' => array(
|
|
|
|
'help' =>
|
|
|
|
'When linting git repositories, amend HEAD with autofix '.
|
|
|
|
'patches suggested by lint without prompting.',
|
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
|
|
|
),
|
2012-11-15 21:33:36 +01:00
|
|
|
'add-all' => array(
|
2013-02-11 19:47:31 +01:00
|
|
|
'short' => 'a',
|
2012-11-15 21:33:36 +01:00
|
|
|
'help' =>
|
2015-04-02 22:40:14 +02:00
|
|
|
'Automatically add all unstaged and uncommitted files to the commit.',
|
2012-11-15 21:33:36 +01:00
|
|
|
),
|
2011-06-23 23:37:43 +02:00
|
|
|
'json' => array(
|
|
|
|
'help' =>
|
|
|
|
'Emit machine-readable JSON. EXPERIMENTAL! Probably does not work!',
|
|
|
|
),
|
2012-02-01 23:33:01 +01:00
|
|
|
'no-amend' => array(
|
2012-11-15 21:33:36 +01:00
|
|
|
'help' => 'Never amend commits in the working copy with lint patches.',
|
2012-02-01 23:33:01 +01:00
|
|
|
),
|
2012-03-20 03:17:10 +01:00
|
|
|
'uncommitted' => array(
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
'help' => 'Suppress warning about uncommitted changes.',
|
2012-03-20 03:17:10 +01:00
|
|
|
'supports' => array(
|
|
|
|
'hg',
|
|
|
|
),
|
|
|
|
),
|
2012-04-10 21:06:41 +02:00
|
|
|
'verbatim' => array(
|
With `--verbatim`, update some fields automatically when updating revisions
Summary:
Essentially D2391, but with, uh, more comments?
- I forgot that we already implemented shouldOverwriteWhenCommitMessageIsEdited(). This patch already behaves nearly correctly.
- Requires changes in D2412.
- Use `'edit' => 'edit'`, which does the same thing as `'edit' => true`, but is more correct after the "edit" / "create" split.
- Under "--verbatim", always get the message "from the user", which means "from the working copy" because verbtatim disables the editor part.
Test Plan:
- Created and updated revisions with `arc diff`.
- Created and updated revisions with `arc diff --verbatim`.
- Updated revisions with `arc diff --edit`.
Reviewers: jungejason, btrahan
Reviewed By: jungejason
CC: vrana, aran
Differential Revision: https://secure.phabricator.com/D2411
2012-05-07 17:16:29 +02:00
|
|
|
'help' => 'When creating a revision, try to use the working copy '.
|
|
|
|
'commit message verbatim, without prompting to edit it. '.
|
|
|
|
'When updating a revision, update some fields from the '.
|
|
|
|
'local commit message.',
|
2012-04-10 21:06:41 +02:00
|
|
|
'supports' => array(
|
|
|
|
'hg',
|
|
|
|
'git',
|
|
|
|
),
|
|
|
|
'conflicts' => array(
|
|
|
|
'use-commit-message' => true,
|
|
|
|
'update' => true,
|
|
|
|
'only' => true,
|
|
|
|
'preview' => true,
|
|
|
|
'raw' => true,
|
|
|
|
'raw-command' => true,
|
|
|
|
'message-file' => true,
|
|
|
|
),
|
|
|
|
),
|
2012-05-22 23:38:53 +02:00
|
|
|
'reviewers' => array(
|
|
|
|
'param' => 'usernames',
|
|
|
|
'help' => 'When creating a revision, add reviewers.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'only' => true,
|
|
|
|
'preview' => true,
|
|
|
|
'update' => true,
|
|
|
|
),
|
|
|
|
),
|
|
|
|
'cc' => array(
|
|
|
|
'param' => 'usernames',
|
|
|
|
'help' => 'When creating a revision, add CCs.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'only' => true,
|
|
|
|
'preview' => true,
|
|
|
|
'update' => true,
|
|
|
|
),
|
|
|
|
),
|
2012-05-30 23:11:07 +02:00
|
|
|
'skip-binaries' => array(
|
|
|
|
'help' => 'Do not upload binaries (like images).',
|
|
|
|
),
|
2012-07-17 00:54:11 +02:00
|
|
|
'ignore-unsound-tests' => array(
|
|
|
|
'help' => 'Ignore unsound test failures without prompting.',
|
|
|
|
),
|
Add a DSL for selecting base commits
Summary:
New optional mode. If you set 'base' in local, project or global config or pass '--base' to 'arc diff' or 'arc which', it switches to DSL mode.
In DSL mode, lists of rules from args, local, project and global config are resolved, in that order. Rules can manipulate the rule machine or resolve into actual commits. Provides support for some 'arc' rules (mostly machine manipulation) and 'git' rules (symbolic ref and merge-base).
Test Plan:
Ran unit tests. Also:
```$ arc which --show-base --base 'arc:prompt'
Against which commit? HEAD
HEAD
$ arc which --show-base --base 'git:HEAD'
HEAD
$ arc which --show-base --base 'git:fake'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'git:origin/master'
origin/master
$ arc which --show-base --base 'git:upstream'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'literal:derp'
derp
$ arc which --show-base --base 'arc:halt'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc set-config --local base git:origin/master
Set key 'base' = 'git:origin/master' in local config.
$ arc which --show-base
origin/master
$ arc which --show-base --base 'git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:yield, git:HEAD^'
origin/master
$ arc which --show-base --base 'arc:global, git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:global, git:merge-base(origin/master)'
3f4f8992fba8d1f142974da36a82bae900e247c0```
Reviewers: dschleimer, vrana
Reviewed By: dschleimer
CC: aran
Maniphest Tasks: T1233
Differential Revision: https://secure.phabricator.com/D2748
2012-06-15 23:01:28 +02:00
|
|
|
'base' => array(
|
|
|
|
'param' => 'rules',
|
|
|
|
'help' => 'Additional rules for determining base revision.',
|
|
|
|
'nosupport' => array(
|
|
|
|
'svn' => 'Subversion does not use base commits.',
|
|
|
|
),
|
|
|
|
'supports' => array('git', 'hg'),
|
|
|
|
),
|
2012-08-06 22:40:20 +02:00
|
|
|
'no-diff' => array(
|
|
|
|
'help' => 'Only run lint and unit tests. Intended for internal use.',
|
|
|
|
),
|
2012-11-21 01:43:10 +01:00
|
|
|
'cache' => array(
|
|
|
|
'param' => 'bool',
|
2014-05-23 22:53:05 +02:00
|
|
|
'help' => '0 to disable lint cache, 1 to enable (default).',
|
2012-11-21 01:43:10 +01:00
|
|
|
'passthru' => array(
|
|
|
|
'lint' => true,
|
|
|
|
),
|
|
|
|
),
|
2013-01-30 21:35:30 +01:00
|
|
|
'coverage' => array(
|
|
|
|
'help' => 'Always enable coverage information.',
|
|
|
|
'conflicts' => array(
|
|
|
|
'no-coverage' => null,
|
|
|
|
),
|
|
|
|
'passthru' => array(
|
|
|
|
'unit' => true,
|
|
|
|
),
|
|
|
|
),
|
|
|
|
'no-coverage' => array(
|
|
|
|
'help' => 'Always disable coverage information.',
|
|
|
|
'passthru' => array(
|
|
|
|
'unit' => true,
|
|
|
|
),
|
|
|
|
),
|
2014-08-04 21:03:03 +02:00
|
|
|
'browse' => array(
|
|
|
|
'help' => pht(
|
|
|
|
'After creating a diff or revision, open it in a web browser.'),
|
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
'*' => 'paths',
|
2014-06-11 23:37:01 +02:00
|
|
|
'head' => array(
|
|
|
|
'param' => 'commit',
|
2014-06-12 01:35:50 +02:00
|
|
|
'help' => pht(
|
|
|
|
'Specify the end of the commit range. This disables many '.
|
|
|
|
'Arcanist/Phabricator features which depend on having access to '.
|
|
|
|
'the working copy.'),
|
2014-06-11 23:37:01 +02:00
|
|
|
'supports' => array('git'),
|
2014-06-12 01:35:50 +02:00
|
|
|
'nosupport' => array(
|
|
|
|
'svn' => pht('Subversion does not support commit ranges.'),
|
|
|
|
'hg' => pht('Mercurial does not support --head yet.'),
|
|
|
|
),
|
2014-06-11 23:37:01 +02:00
|
|
|
'conflicts' => array(
|
|
|
|
'lintall' => '--head suppresses lint.',
|
|
|
|
'advice' => '--head suppresses lint.',
|
|
|
|
),
|
2014-09-27 02:21:23 +02:00
|
|
|
),
|
2011-01-10 00:22:25 +01:00
|
|
|
);
|
2012-09-18 23:14:12 +02:00
|
|
|
|
|
|
|
return $arguments;
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
2012-01-05 22:36:24 +01:00
|
|
|
public function isRawDiffSource() {
|
|
|
|
return $this->getArgument('raw') || $this->getArgument('raw-command');
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
public function run() {
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console = PhutilConsole::getConsole();
|
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
$this->runRepositoryAPISetup();
|
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
if ($this->getArgument('no-diff')) {
|
|
|
|
$this->removeScratchFile('diff-result.json');
|
|
|
|
$data = $this->runLintUnit();
|
2012-08-08 21:08:41 +02:00
|
|
|
$this->writeScratchJSONFile('diff-result.json', $data);
|
2012-08-06 22:40:20 +02:00
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
2012-02-20 21:51:01 +01:00
|
|
|
$this->runDiffSetupBasics();
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$commit_message = $this->buildCommitMessage();
|
2011-03-13 02:57:35 +01:00
|
|
|
|
2012-09-04 21:08:04 +02:00
|
|
|
$this->dispatchEvent(
|
|
|
|
ArcanistEventType::TYPE_DIFF_DIDBUILDMESSAGE,
|
2013-01-09 00:06:46 +01:00
|
|
|
array(
|
2013-01-09 00:51:32 +01:00
|
|
|
'message' => $commit_message,
|
|
|
|
));
|
2012-08-28 03:21:49 +02:00
|
|
|
|
2012-08-08 21:08:41 +02:00
|
|
|
if (!$this->shouldOnlyCreateDiff()) {
|
|
|
|
$revision = $this->buildRevisionFromCommitMessage($commit_message);
|
|
|
|
}
|
|
|
|
|
Remove --background flag to prevent `arc` from hanging
Summary:
Ref T4281. A long time ago, we added a `--background` flag to let `arc lint` and `arc unit` run while you're typing a commit message, in some situations.
This code is only moderately beneficial and is way too complicated. Particularly, it has a long history of causing hangs (T4281, T2463), doesn't work on Windows, and is impossible to debug.
It's also running into a serious PHP bug with EAGAIN/EPIPE being indistinguishable that I haven't been able to find a reasonable workaround for in ~3-4 hours of trying.
All the pathways forward that I can see make this already-complex system more complex.
The major reason that this stuff is so complex is that the subprocess may need to prompt the user (notably, to apply patches from lint).
Instead, I'm going to simplify how `arc diff` interacts with `arc lint` and `arc unit`, so we can just fire-and-forget a background process, let it do as much work as it can without needing user input, and then pick up wherever it left off. This will be slightly less cool/magical, but it won't hang bizarrely and I will be able to debug it.
For now, simply remove the `--background` flag and behavior so `arc` works for everyone.
Test Plan: Ran `arc diff` to create this diff.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4281
Differential Revision: https://secure.phabricator.com/D10198
2014-08-09 01:09:11 +02:00
|
|
|
$server = $this->console->getServer();
|
|
|
|
$server->setHandler(array($this, 'handleServerMessage'));
|
|
|
|
$data = $this->runLintUnit();
|
2012-08-06 22:40:20 +02:00
|
|
|
|
|
|
|
$lint_result = $data['lintResult'];
|
|
|
|
$this->unresolvedLint = $data['unresolvedLint'];
|
|
|
|
$this->postponedLinters = $data['postponedLinters'];
|
|
|
|
$unit_result = $data['unitResult'];
|
|
|
|
$this->testResults = $data['testResults'];
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-11-02 06:41:00 +01:00
|
|
|
if ($this->getArgument('nolint')) {
|
|
|
|
$this->excuses['lint'] = $this->getSkipExcuse(
|
|
|
|
'Provide explanation for skipping lint or press Enter to abort:',
|
|
|
|
'lint-excuses');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->getArgument('nounit')) {
|
|
|
|
$this->excuses['unit'] = $this->getSkipExcuse(
|
|
|
|
'Provide explanation for skipping unit tests or press Enter to abort:',
|
|
|
|
'unit-excuses');
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
$changes = $this->generateChanges();
|
|
|
|
if (!$changes) {
|
|
|
|
throw new ArcanistUsageException(
|
2014-05-23 22:53:05 +02:00
|
|
|
'There are no changes to generate a diff from!');
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$diff_spec = array(
|
Allow `arc` to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:
- Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
- Enhance `arc which` to explain the process to the user.
- The `project_id` key is no longer required in `.arcconfig`.
Minor/cleanup changes:
- Rename `project_id` to `project.name` (consistency, clarity).
- Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
- These both need documentation updates.
- Add `repository.callsign` to explicitly bind to a repository.
- Updated `.arcconfig` for the new values.
- Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
- Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.
Test Plan:
- Ran `arc which`.
- Ran `arc diff`.
- This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4343
Differential Revision: https://secure.phabricator.com/D8073
2014-01-27 00:31:30 +01:00
|
|
|
'changes' => mpull($changes, 'toDictionary'),
|
|
|
|
'lintStatus' => $this->getLintStatus($lint_result),
|
|
|
|
'unitStatus' => $this->getUnitStatus($unit_result),
|
2012-01-05 20:26:17 +01:00
|
|
|
) + $this->buildDiffSpecification();
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-02-20 21:51:01 +01:00
|
|
|
$conduit = $this->getConduit();
|
2011-01-10 00:22:25 +01:00
|
|
|
$diff_info = $conduit->callMethodSynchronous(
|
|
|
|
'differential.creatediff',
|
2012-01-05 20:26:17 +01:00
|
|
|
$diff_spec);
|
|
|
|
|
|
|
|
$this->diffID = $diff_info['diffid'];
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-09-04 21:08:04 +02:00
|
|
|
$event = $this->dispatchEvent(
|
2012-08-13 21:20:34 +02:00
|
|
|
ArcanistEventType::TYPE_DIFF_WASCREATED,
|
|
|
|
array(
|
|
|
|
'diffID' => $diff_info['diffid'],
|
|
|
|
'lintResult' => $lint_result,
|
|
|
|
'unitResult' => $unit_result,
|
|
|
|
));
|
2012-07-03 00:53:22 +02:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$this->updateLintDiffProperty();
|
|
|
|
$this->updateUnitDiffProperty();
|
|
|
|
$this->updateLocalDiffProperty();
|
2012-10-13 00:01:19 +02:00
|
|
|
$this->resolveDiffPropertyUpdates();
|
2011-01-12 07:13:31 +01:00
|
|
|
|
2012-02-20 21:51:01 +01:00
|
|
|
$output_json = $this->getArgument('json');
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
if ($this->shouldOnlyCreateDiff()) {
|
2011-06-23 23:37:43 +02:00
|
|
|
if (!$output_json) {
|
|
|
|
echo phutil_console_format(
|
|
|
|
"Created a new Differential diff:\n".
|
|
|
|
" **Diff URI:** __%s__\n\n",
|
|
|
|
$diff_info['uri']);
|
|
|
|
} else {
|
|
|
|
$human = ob_get_clean();
|
|
|
|
echo json_encode(array(
|
|
|
|
'diffURI' => $diff_info['uri'],
|
2012-01-05 20:26:17 +01:00
|
|
|
'diffID' => $this->getDiffID(),
|
2011-06-23 23:37:43 +02:00
|
|
|
'human' => $human,
|
|
|
|
))."\n";
|
|
|
|
ob_start();
|
|
|
|
}
|
2014-08-04 21:03:03 +02:00
|
|
|
|
|
|
|
if ($this->shouldOpenCreatedObjectsInBrowser()) {
|
|
|
|
$this->openURIsInBrowser(array($diff_info['uri']));
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
} else {
|
2012-08-08 21:08:41 +02:00
|
|
|
$revision['diffid'] = $this->getDiffID();
|
2012-02-23 18:18:49 +01:00
|
|
|
|
2012-08-08 21:08:41 +02:00
|
|
|
if ($commit_message->getRevisionID()) {
|
2012-12-05 23:04:10 +01:00
|
|
|
$result = $conduit->callMethodSynchronous(
|
2011-01-10 00:22:25 +01:00
|
|
|
'differential.updaterevision',
|
|
|
|
$revision);
|
2012-08-08 21:08:41 +02:00
|
|
|
|
|
|
|
foreach (array('edit-messages.json', 'update-messages.json') as $file) {
|
|
|
|
$messages = $this->readScratchJSONFile($file);
|
|
|
|
unset($messages[$revision['id']]);
|
|
|
|
$this->writeScratchJSONFile($file, $messages);
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
echo "Updated an existing Differential revision:\n";
|
|
|
|
} else {
|
2012-08-30 21:28:35 +02:00
|
|
|
$revision = $this->dispatchWillCreateRevisionEvent($revision);
|
|
|
|
|
2012-10-13 00:01:19 +02:00
|
|
|
$result = $conduit->callMethodSynchronous(
|
2011-01-10 00:22:25 +01:00
|
|
|
'differential.createrevision',
|
|
|
|
$revision);
|
2011-12-21 00:05:46 +01:00
|
|
|
|
|
|
|
$revised_message = $conduit->callMethodSynchronous(
|
|
|
|
'differential.getcommitmessage',
|
|
|
|
array(
|
|
|
|
'revision_id' => $result['revisionid'],
|
|
|
|
));
|
|
|
|
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
if ($this->shouldAmend()) {
|
2012-01-05 22:36:24 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
2012-06-04 21:30:50 +02:00
|
|
|
if ($repository_api->supportsAmend()) {
|
2012-01-05 22:36:24 +01:00
|
|
|
echo "Updating commit message...\n";
|
2012-06-04 21:30:50 +02:00
|
|
|
$repository_api->amendCommit($revised_message);
|
|
|
|
} else {
|
2014-05-23 22:53:05 +02:00
|
|
|
echo 'Commit message was not amended. Amending commit message is '.
|
|
|
|
'only supported in git and hg (version 2.2 or newer)';
|
2012-01-05 22:36:24 +01:00
|
|
|
}
|
|
|
|
}
|
2011-12-21 00:05:46 +01:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
echo "Created a new Differential revision:\n";
|
|
|
|
}
|
|
|
|
|
|
|
|
$uri = $result['uri'];
|
|
|
|
echo phutil_console_format(
|
|
|
|
" **Revision URI:** __%s__\n\n",
|
|
|
|
$uri);
|
2012-12-05 23:04:10 +01:00
|
|
|
|
|
|
|
if ($this->getArgument('plan-changes')) {
|
|
|
|
$conduit->callMethodSynchronous(
|
|
|
|
'differential.createcomment',
|
|
|
|
array(
|
|
|
|
'revision_id' => $result['revisionid'],
|
|
|
|
'action' => 'rethink',
|
|
|
|
));
|
|
|
|
echo "Planned changes to the revision.\n";
|
|
|
|
}
|
2014-08-04 21:03:03 +02:00
|
|
|
|
|
|
|
if ($this->shouldOpenCreatedObjectsInBrowser()) {
|
|
|
|
$this->openURIsInBrowser(array($uri));
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
echo "Included changes:\n";
|
|
|
|
foreach ($changes as $change) {
|
|
|
|
echo ' '.$change->renderTextSummary()."\n";
|
|
|
|
}
|
2011-01-30 02:18:32 +01:00
|
|
|
|
2011-06-23 23:37:43 +02:00
|
|
|
if ($output_json) {
|
|
|
|
ob_get_clean();
|
|
|
|
}
|
|
|
|
|
2012-02-21 21:35:39 +01:00
|
|
|
$this->removeScratchFile('create-message');
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
private function runRepositoryAPISetup() {
|
|
|
|
if (!$this->requiresRepositoryAPI()) {
|
|
|
|
return;
|
|
|
|
}
|
2012-03-23 01:22:52 +01:00
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
if ($this->getArgument('less-context')) {
|
|
|
|
$repository_api->setDiffLinesOfContext(3);
|
|
|
|
}
|
Add a DSL for selecting base commits
Summary:
New optional mode. If you set 'base' in local, project or global config or pass '--base' to 'arc diff' or 'arc which', it switches to DSL mode.
In DSL mode, lists of rules from args, local, project and global config are resolved, in that order. Rules can manipulate the rule machine or resolve into actual commits. Provides support for some 'arc' rules (mostly machine manipulation) and 'git' rules (symbolic ref and merge-base).
Test Plan:
Ran unit tests. Also:
```$ arc which --show-base --base 'arc:prompt'
Against which commit? HEAD
HEAD
$ arc which --show-base --base 'git:HEAD'
HEAD
$ arc which --show-base --base 'git:fake'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'git:origin/master'
origin/master
$ arc which --show-base --base 'git:upstream'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'literal:derp'
derp
$ arc which --show-base --base 'arc:halt'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc set-config --local base git:origin/master
Set key 'base' = 'git:origin/master' in local config.
$ arc which --show-base
origin/master
$ arc which --show-base --base 'git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:yield, git:HEAD^'
origin/master
$ arc which --show-base --base 'arc:global, git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:global, git:merge-base(origin/master)'
3f4f8992fba8d1f142974da36a82bae900e247c0```
Reviewers: dschleimer, vrana
Reviewed By: dschleimer
CC: aran
Maniphest Tasks: T1233
Differential Revision: https://secure.phabricator.com/D2748
2012-06-15 23:01:28 +02:00
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
$repository_api->setBaseCommitArgumentRules(
|
|
|
|
$this->getArgument('base', ''));
|
2012-03-23 01:22:52 +01:00
|
|
|
|
2012-12-17 21:54:08 +01:00
|
|
|
if ($repository_api->supportsCommitRanges()) {
|
|
|
|
$this->parseBaseCommitArgument($this->getArgument('paths'));
|
2012-02-20 21:51:01 +01:00
|
|
|
}
|
2014-09-08 23:02:25 +02:00
|
|
|
|
|
|
|
$head_commit = $this->getArgument('head');
|
|
|
|
if ($head_commit !== null) {
|
|
|
|
$repository_api->setHeadCommit($head_commit);
|
|
|
|
}
|
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
}
|
2012-02-20 21:51:01 +01:00
|
|
|
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
private function runDiffSetupBasics() {
|
2012-02-20 21:51:01 +01:00
|
|
|
$output_json = $this->getArgument('json');
|
|
|
|
if ($output_json) {
|
|
|
|
// TODO: We should move this to a higher-level and put an indirection
|
|
|
|
// layer between echoing stuff and stdout.
|
|
|
|
ob_start();
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->requiresWorkingCopy()) {
|
2012-12-15 00:49:01 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
2015-05-03 02:07:15 +02:00
|
|
|
if ($this->getArgument('add-all')) {
|
|
|
|
$this->setCommitMode(self::COMMIT_ENABLE);
|
|
|
|
} else if ($this->getArgument('uncommitted')) {
|
|
|
|
$this->setCommitMode(self::COMMIT_DISABLE);
|
|
|
|
} else {
|
|
|
|
$this->setCommitMode(self::COMMIT_ALLOW);
|
|
|
|
}
|
|
|
|
if ($repository_api instanceof ArcanistSubversionAPI) {
|
|
|
|
$repository_api->limitStatusToPaths($this->getArgument('paths'));
|
|
|
|
}
|
|
|
|
if (!$this->getArgument('head')) {
|
|
|
|
$this->requireCleanWorkingCopy();
|
2012-03-20 03:17:10 +01:00
|
|
|
}
|
2012-02-20 21:51:01 +01:00
|
|
|
}
|
2013-02-01 23:59:09 +01:00
|
|
|
|
|
|
|
$this->dispatchEvent(
|
|
|
|
ArcanistEventType::TYPE_DIFF_DIDCOLLECTCHANGES,
|
|
|
|
array());
|
2012-02-20 21:51:01 +01:00
|
|
|
}
|
|
|
|
|
2012-10-16 20:51:15 +02:00
|
|
|
private function buildRevisionFromCommitMessage(
|
|
|
|
ArcanistDifferentialCommitMessage $message) {
|
|
|
|
|
2012-08-08 21:08:41 +02:00
|
|
|
$conduit = $this->getConduit();
|
|
|
|
|
|
|
|
$revision_id = $message->getRevisionID();
|
|
|
|
$revision = array(
|
|
|
|
'fields' => $message->getFields(),
|
|
|
|
);
|
|
|
|
|
|
|
|
if ($revision_id) {
|
|
|
|
|
|
|
|
// With '--verbatim', pass the (possibly modified) local fields. This
|
|
|
|
// allows the user to edit some fields (like "title" and "summary")
|
|
|
|
// locally without '--edit' and have changes automatically synchronized.
|
|
|
|
// Without '--verbatim', we do not update the revision to reflect local
|
|
|
|
// commit message changes.
|
|
|
|
if ($this->getArgument('verbatim')) {
|
|
|
|
$use_fields = $message->getFields();
|
|
|
|
} else {
|
|
|
|
$use_fields = array();
|
|
|
|
}
|
|
|
|
|
|
|
|
$should_edit = $this->getArgument('edit');
|
|
|
|
$edit_messages = $this->readScratchJSONFile('edit-messages.json');
|
|
|
|
$remote_corpus = idx($edit_messages, $revision_id);
|
|
|
|
|
|
|
|
if (!$should_edit || !$remote_corpus || $use_fields) {
|
2012-10-16 20:51:15 +02:00
|
|
|
if ($this->commitMessageFromRevision) {
|
|
|
|
$remote_corpus = $this->commitMessageFromRevision;
|
|
|
|
} else {
|
|
|
|
$remote_corpus = $conduit->callMethodSynchronous(
|
|
|
|
'differential.getcommitmessage',
|
|
|
|
array(
|
|
|
|
'revision_id' => $revision_id,
|
|
|
|
'edit' => 'edit',
|
|
|
|
'fields' => $use_fields,
|
|
|
|
));
|
|
|
|
}
|
2012-08-08 21:08:41 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($should_edit) {
|
2012-10-16 20:51:15 +02:00
|
|
|
$edited = $this->newInteractiveEditor($remote_corpus)
|
2012-08-08 21:08:41 +02:00
|
|
|
->setName('differential-edit-revision-info')
|
|
|
|
->editInteractively();
|
2012-10-16 20:51:15 +02:00
|
|
|
if ($edited != $remote_corpus) {
|
|
|
|
$remote_corpus = $edited;
|
|
|
|
$edit_messages[$revision_id] = $remote_corpus;
|
|
|
|
$this->writeScratchJSONFile('edit-messages.json', $edit_messages);
|
|
|
|
}
|
2012-08-08 21:08:41 +02:00
|
|
|
}
|
|
|
|
|
2012-10-16 20:51:15 +02:00
|
|
|
if ($this->commitMessageFromRevision == $remote_corpus) {
|
|
|
|
$new_message = $message;
|
|
|
|
} else {
|
2014-04-01 17:21:05 +02:00
|
|
|
$remote_corpus = ArcanistCommentRemover::removeComments(
|
|
|
|
$remote_corpus);
|
2012-10-16 20:51:15 +02:00
|
|
|
$new_message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
|
|
|
|
$remote_corpus);
|
|
|
|
$new_message->pullDataFromConduit($conduit);
|
|
|
|
}
|
2012-08-08 21:08:41 +02:00
|
|
|
|
|
|
|
$revision['fields'] = $new_message->getFields();
|
|
|
|
|
|
|
|
$revision['id'] = $revision_id;
|
|
|
|
$this->revisionID = $revision_id;
|
|
|
|
|
|
|
|
$revision['message'] = $this->getArgument('message');
|
|
|
|
if (!strlen($revision['message'])) {
|
|
|
|
$update_messages = $this->readScratchJSONFile('update-messages.json');
|
|
|
|
|
|
|
|
$update_messages[$revision_id] = $this->getUpdateMessage(
|
|
|
|
$revision['fields'],
|
|
|
|
idx($update_messages, $revision_id));
|
|
|
|
|
|
|
|
$revision['message'] = ArcanistCommentRemover::removeComments(
|
|
|
|
$update_messages[$revision_id]);
|
|
|
|
if (!strlen(trim($revision['message']))) {
|
|
|
|
throw new ArcanistUserAbortException();
|
|
|
|
}
|
|
|
|
|
|
|
|
$this->writeScratchJSONFile('update-messages.json', $update_messages);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $revision;
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
protected function shouldOnlyCreateDiff() {
|
Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:00:29 +02:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
if ($this->getArgument('create')) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2012-01-09 18:33:55 +01:00
|
|
|
if ($this->getArgument('update')) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2012-01-13 04:03:11 +01:00
|
|
|
if ($this->getArgument('use-commit-message')) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2012-01-05 22:36:24 +01:00
|
|
|
if ($this->isRawDiffSource()) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return $this->getArgument('preview') ||
|
|
|
|
$this->getArgument('only');
|
|
|
|
}
|
|
|
|
|
|
|
|
private function generateAffectedPaths() {
|
2012-01-05 22:36:24 +01:00
|
|
|
if ($this->isRawDiffSource()) {
|
|
|
|
return array();
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
if ($repository_api instanceof ArcanistSubversionAPI) {
|
|
|
|
$file_list = new FileList($this->getArgument('paths', array()));
|
|
|
|
$paths = $repository_api->getSVNStatus($externals = true);
|
|
|
|
foreach ($paths as $path => $mask) {
|
|
|
|
if (!$file_list->contains($repository_api->getPath($path), true)) {
|
|
|
|
unset($paths[$path]);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$warn_externals = array();
|
|
|
|
foreach ($paths as $path => $mask) {
|
|
|
|
$any_mod = ($mask & ArcanistRepositoryAPI::FLAG_ADDED) ||
|
|
|
|
($mask & ArcanistRepositoryAPI::FLAG_MODIFIED) ||
|
|
|
|
($mask & ArcanistRepositoryAPI::FLAG_DELETED);
|
|
|
|
if ($mask & ArcanistRepositoryAPI::FLAG_EXTERNALS) {
|
|
|
|
unset($paths[$path]);
|
|
|
|
if ($any_mod) {
|
|
|
|
$warn_externals[] = $path;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($warn_externals && !$this->hasWarnedExternals) {
|
|
|
|
echo phutil_console_format(
|
|
|
|
"The working copy includes changes to 'svn:externals' paths. These ".
|
|
|
|
"changes will not be included in the diff because SVN can not ".
|
|
|
|
"commit 'svn:externals' changes alongside normal changes.".
|
|
|
|
"\n\n".
|
|
|
|
"Modified 'svn:externals' files:".
|
|
|
|
"\n\n".
|
2012-03-29 06:38:31 +02:00
|
|
|
phutil_console_wrap(implode("\n", $warn_externals), 8));
|
2014-05-23 22:53:05 +02:00
|
|
|
$prompt = 'Generate a diff (with just local changes) anyway?';
|
2011-01-10 00:22:25 +01:00
|
|
|
if (!phutil_console_confirm($prompt)) {
|
|
|
|
throw new ArcanistUserAbortException();
|
|
|
|
} else {
|
|
|
|
$this->hasWarnedExternals = true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:00:29 +02:00
|
|
|
} else {
|
2012-12-17 21:54:08 +01:00
|
|
|
$paths = $repository_api->getWorkingCopyStatus();
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
2011-02-19 01:52:27 +01:00
|
|
|
|
|
|
|
foreach ($paths as $path => $mask) {
|
|
|
|
if ($mask & ArcanistRepositoryAPI::FLAG_UNTRACKED) {
|
|
|
|
unset($paths[$path]);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return $paths;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
protected function generateChanges() {
|
2012-07-18 01:16:18 +02:00
|
|
|
$parser = $this->newDiffParser();
|
2012-01-05 22:36:24 +01:00
|
|
|
|
|
|
|
$is_raw = $this->isRawDiffSource();
|
|
|
|
if ($is_raw) {
|
|
|
|
|
|
|
|
if ($this->getArgument('raw')) {
|
2012-10-09 01:23:49 +02:00
|
|
|
fwrite(STDERR, "Reading diff from stdin...\n");
|
2012-01-05 22:36:24 +01:00
|
|
|
$raw_diff = file_get_contents('php://stdin');
|
|
|
|
} else if ($this->getArgument('raw-command')) {
|
2013-02-02 22:07:39 +01:00
|
|
|
list($raw_diff) = execx('%C', $this->getArgument('raw-command'));
|
2012-01-05 22:36:24 +01:00
|
|
|
} else {
|
2014-05-23 22:53:05 +02:00
|
|
|
throw new Exception('Unknown raw diff source.');
|
2012-01-05 22:36:24 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
$changes = $parser->parseDiff($raw_diff);
|
|
|
|
foreach ($changes as $key => $change) {
|
|
|
|
// Remove "message" changes, e.g. from "git show".
|
|
|
|
if ($change->getType() == ArcanistDiffChangeType::TYPE_MESSAGE) {
|
|
|
|
unset($changes[$key]);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return $changes;
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
|
|
|
if ($repository_api instanceof ArcanistSubversionAPI) {
|
|
|
|
$paths = $this->generateAffectedPaths();
|
|
|
|
$this->primeSubversionWorkingCopyData($paths);
|
|
|
|
|
|
|
|
// Check to make sure the user is diffing from a consistent base revision.
|
|
|
|
// This is mostly just an abuse sanity check because it's silly to do this
|
|
|
|
// and makes the code more difficult to effectively review, but it also
|
|
|
|
// affects patches and makes them nonportable.
|
|
|
|
$bases = $repository_api->getSVNBaseRevisions();
|
|
|
|
|
|
|
|
// Remove all files with baserev "0"; these files are new.
|
|
|
|
foreach ($bases as $path => $baserev) {
|
2012-03-13 00:18:44 +01:00
|
|
|
if ($bases[$path] <= 0) {
|
2011-01-10 00:22:25 +01:00
|
|
|
unset($bases[$path]);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($bases) {
|
2011-02-23 21:06:22 +01:00
|
|
|
$rev = reset($bases);
|
2011-06-23 23:37:43 +02:00
|
|
|
|
|
|
|
$revlist = array();
|
|
|
|
foreach ($bases as $path => $baserev) {
|
|
|
|
$revlist[] = " Revision {$baserev}, {$path}";
|
|
|
|
}
|
|
|
|
$revlist = implode("\n", $revlist);
|
|
|
|
|
2011-02-23 21:06:22 +01:00
|
|
|
foreach ($bases as $path => $baserev) {
|
|
|
|
if ($baserev !== $rev) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
"Base revisions of changed paths are mismatched. Update all ".
|
|
|
|
"paths to the same base revision before creating a diff: ".
|
|
|
|
"\n\n".
|
|
|
|
$revlist);
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
}
|
2011-02-25 01:34:27 +01:00
|
|
|
|
2011-02-23 21:06:22 +01:00
|
|
|
// If you have a change which affects several files, all of which are
|
|
|
|
// at a consistent base revision, treat that revision as the effective
|
|
|
|
// base revision. The use case here is that you made a change to some
|
|
|
|
// file, which updates it to HEAD, but want to be able to change it
|
|
|
|
// again without updating the entire working copy. This is a little
|
|
|
|
// sketchy but it arises in Facebook Ops workflows with config files and
|
|
|
|
// doesn't have any real material tradeoffs (e.g., these patches are
|
|
|
|
// perfectly applyable).
|
|
|
|
$repository_api->overrideSVNBaseRevisionNumber($rev);
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
$changes = $parser->parseSubversionDiff(
|
|
|
|
$repository_api,
|
|
|
|
$paths);
|
|
|
|
} else if ($repository_api instanceof ArcanistGitAPI) {
|
2014-06-11 23:37:01 +02:00
|
|
|
$diff = $repository_api->getFullGitDiff(
|
|
|
|
$repository_api->getBaseCommit(),
|
|
|
|
$repository_api->getHeadCommit());
|
2011-01-10 00:22:25 +01:00
|
|
|
if (!strlen($diff)) {
|
Improve git behavior in the zero- and one- commit case
Summary:
Git works completely differently for commits zero and one than for 2..N so add
more special casing to handle them. See:
- {T206}
- {T596}
The getCommitRange() block is also fatal land, although I wasn't able to reach
it. I'll follow up with @s on T596.
Test Plan:
- Created a new, empty repository ("mkdir x; cd x; git init").
- Ran "arc lint", "arc unit", "arc diff" against it with no commits (the first
two work, the third fails helpfully).
- Made an initial commit.
- Ran "arc lint", "arc unit", "arc diff" against it (all work correctly).
Reviewers: btrahan, jungejason, aran
Reviewed By: aran
CC: s, aran
Differential Revision: 1142
2011-11-30 18:15:37 +01:00
|
|
|
throw new ArcanistUsageException(
|
2014-05-23 22:53:05 +02:00
|
|
|
'No changes found. (Did you specify the wrong commit range?)');
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
$changes = $parser->parseDiff($diff);
|
Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:00:29 +02:00
|
|
|
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
|
|
|
$diff = $repository_api->getFullMercurialDiff();
|
2012-03-20 03:17:10 +01:00
|
|
|
if (!strlen($diff)) {
|
|
|
|
throw new ArcanistUsageException(
|
2014-05-23 22:53:05 +02:00
|
|
|
'No changes found. (Did you specify the wrong commit range?)');
|
2012-03-20 03:17:10 +01:00
|
|
|
}
|
Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:00:29 +02:00
|
|
|
$changes = $parser->parseDiff($diff);
|
2011-01-10 00:22:25 +01:00
|
|
|
} else {
|
2014-05-23 22:53:05 +02:00
|
|
|
throw new Exception('Repository API is not supported.');
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if (count($changes) > 250) {
|
|
|
|
$count = number_format(count($changes));
|
2012-08-16 23:27:35 +02:00
|
|
|
$link =
|
2014-05-23 22:53:05 +02:00
|
|
|
'http://www.phabricator.com/docs/phabricator/article/'.
|
|
|
|
'Differential_User_Guide_Large_Changes.html';
|
2011-01-10 00:22:25 +01:00
|
|
|
$message =
|
|
|
|
"This diff has a very large number of changes ({$count}). ".
|
|
|
|
"Differential works best for changes which will receive detailed ".
|
|
|
|
"human review, and not as well for large automated changes or ".
|
2012-08-16 23:27:35 +02:00
|
|
|
"bulk checkins. See {$link} for information about reviewing big ".
|
|
|
|
"checkins. Continue anyway?";
|
2011-01-10 00:22:25 +01:00
|
|
|
if (!phutil_console_confirm($message)) {
|
|
|
|
throw new ArcanistUsageException(
|
2014-05-23 22:53:05 +02:00
|
|
|
'Aborted generation of gigantic diff.');
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$limit = 1024 * 1024 * 4;
|
|
|
|
foreach ($changes as $change) {
|
|
|
|
$size = 0;
|
|
|
|
foreach ($change->getHunks() as $hunk) {
|
|
|
|
$size += strlen($hunk->getCorpus());
|
|
|
|
}
|
|
|
|
if ($size > $limit) {
|
|
|
|
$file_name = $change->getCurrentPath();
|
|
|
|
$change_size = number_format($size);
|
|
|
|
$byte_warning =
|
|
|
|
"Diff for '{$file_name}' with context is {$change_size} bytes in ".
|
2012-04-14 01:21:23 +02:00
|
|
|
"length. Generally, source changes should not be this large.";
|
|
|
|
if (!$this->getArgument('less-context')) {
|
|
|
|
$byte_warning .=
|
|
|
|
" If this file is a huge text file, try using the ".
|
|
|
|
"'--less-context' flag.";
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
if ($repository_api instanceof ArcanistSubversionAPI) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
"{$byte_warning} If the file is not a text file, mark it as ".
|
|
|
|
"binary with:".
|
|
|
|
"\n\n".
|
|
|
|
" $ svn propset svn:mime-type application/octet-stream <filename>".
|
|
|
|
"\n");
|
|
|
|
} else {
|
|
|
|
$confirm =
|
|
|
|
"{$byte_warning} If the file is not a text file, you can ".
|
|
|
|
"mark it 'binary'. Mark this file as 'binary' and continue?";
|
|
|
|
if (phutil_console_confirm($confirm)) {
|
When converting a file to a binary, populate the binary's data
Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.
A simple reproduction case is to build a test file (I used one with bytes from 1..255):
$ # Don't include \0, since Git treats that specially.
$ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt
Then add it:
$ git add example.txt
$ git commit -a -m derp
$ arc diff --only HEAD^
You'll be prompted to convert the file to binary:
Do you want to mark this file as binary and continue? [Y/n] y
Before this patch, that would be followed by:
Uploading 0 files...
...which is incorrect; we need to upload the new data. After this patch, this shows:
Uploading 1 files...
...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.
Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6815
2013-08-27 18:34:30 +02:00
|
|
|
$change->convertToBinaryChange($repository_api);
|
2011-01-10 00:22:25 +01:00
|
|
|
} else {
|
|
|
|
throw new ArcanistUsageException(
|
2014-05-23 22:53:05 +02:00
|
|
|
'Aborted generation of gigantic diff.');
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2012-03-14 15:08:06 +01:00
|
|
|
$try_encoding = nonempty($this->getArgument('encoding'), null);
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
|
2011-05-21 16:16:12 +02:00
|
|
|
$utf8_problems = array();
|
|
|
|
foreach ($changes as $change) {
|
|
|
|
foreach ($change->getHunks() as $hunk) {
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
$corpus = $hunk->getCorpus();
|
|
|
|
if (!phutil_is_utf8($corpus)) {
|
|
|
|
|
|
|
|
// If this corpus is heuristically binary, don't try to convert it.
|
|
|
|
// mb_check_encoding() and mb_convert_encoding() are both very very
|
|
|
|
// liberal about what they're willing to process.
|
|
|
|
$is_binary = ArcanistDiffUtils::isHeuristicBinaryFile($corpus);
|
|
|
|
if (!$is_binary) {
|
2012-03-14 15:08:06 +01:00
|
|
|
|
|
|
|
if (!$try_encoding) {
|
2011-12-01 17:56:03 +01:00
|
|
|
try {
|
2012-03-14 15:08:06 +01:00
|
|
|
$try_encoding = $this->getRepositoryEncoding();
|
2011-12-01 17:56:03 +01:00
|
|
|
} catch (ConduitClientException $e) {
|
2012-03-14 15:08:06 +01:00
|
|
|
if ($e->getErrorCode() == 'ERR-BAD-ARCANIST-PROJECT') {
|
|
|
|
echo phutil_console_wrap(
|
|
|
|
"Lookup of encoding in arcanist project failed\n".
|
|
|
|
$e->getMessage());
|
|
|
|
} else {
|
|
|
|
throw $e;
|
|
|
|
}
|
2011-12-01 17:56:03 +01:00
|
|
|
}
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
}
|
2012-03-14 15:08:06 +01:00
|
|
|
|
2012-08-12 17:50:01 +02:00
|
|
|
if ($try_encoding) {
|
|
|
|
$corpus = phutil_utf8_convert($corpus, 'UTF-8', $try_encoding);
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
$name = $change->getCurrentPath();
|
|
|
|
if (phutil_is_utf8($corpus)) {
|
|
|
|
$this->writeStatusMessage(
|
2012-07-03 04:34:59 +02:00
|
|
|
"Converted a '{$name}' hunk from '{$try_encoding}' ".
|
|
|
|
"to UTF-8.\n");
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
$hunk->setCorpus($corpus);
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2011-05-21 16:16:12 +02:00
|
|
|
$utf8_problems[] = $change;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// If there are non-binary files which aren't valid UTF-8, warn the user
|
|
|
|
// and treat them as binary changes. See D327 for discussion of why Arcanist
|
|
|
|
// has this behavior.
|
|
|
|
if ($utf8_problems) {
|
2012-06-12 03:33:19 +02:00
|
|
|
$utf8_warning =
|
|
|
|
pht(
|
2015-02-10 08:45:30 +01:00
|
|
|
'This diff includes %s file(s) which are not valid UTF-8 (they '.
|
|
|
|
'contain invalid byte sequences). You can either stop this workflow '.
|
|
|
|
'and fix these files, or continue. If you continue, these files '.
|
|
|
|
'will be marked as binary.',
|
|
|
|
new PhutilNumber(count($utf8_problems))).
|
|
|
|
"\n\n".
|
Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.
This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.
See T452 for a broader discussion of the issues involved.
Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.
@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Reviewed By: davidreuss
CC: aran, davidreuss, epriestley, nshamg123
Differential Revision: 812
2011-08-15 18:10:22 +02:00
|
|
|
"You can learn more about how Phabricator handles character encodings ".
|
|
|
|
"(and how to configure encoding settings and detect and correct ".
|
|
|
|
"encoding problems) by reading 'User Guide: UTF-8 and Character ".
|
2013-10-20 16:41:48 +02:00
|
|
|
"Encoding' in the Phabricator documentation.\n\n".
|
2015-02-10 08:45:30 +01:00
|
|
|
" ".pht('%d AFFECTED FILE(S)', count($utf8_problems))."\n";
|
2012-06-12 03:33:19 +02:00
|
|
|
$confirm = pht(
|
2015-02-10 08:45:30 +01:00
|
|
|
'Do you want to mark these %s file(s) as binary and continue?',
|
|
|
|
new PhutilNumber(count($utf8_problems)));
|
2011-05-21 16:16:12 +02:00
|
|
|
|
|
|
|
echo phutil_console_format("**Invalid Content Encoding (Non-UTF8)**\n");
|
|
|
|
echo phutil_console_wrap($utf8_warning);
|
|
|
|
|
|
|
|
$file_list = mpull($utf8_problems, 'getCurrentPath');
|
|
|
|
$file_list = ' '.implode("\n ", $file_list);
|
|
|
|
echo $file_list;
|
|
|
|
|
|
|
|
if (!phutil_console_confirm($confirm, $default_no = false)) {
|
2014-05-23 22:53:05 +02:00
|
|
|
throw new ArcanistUsageException('Aborted workflow to fix UTF-8.');
|
2011-05-21 16:16:12 +02:00
|
|
|
} else {
|
|
|
|
foreach ($utf8_problems as $change) {
|
When converting a file to a binary, populate the binary's data
Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.
A simple reproduction case is to build a test file (I used one with bytes from 1..255):
$ # Don't include \0, since Git treats that specially.
$ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt
Then add it:
$ git add example.txt
$ git commit -a -m derp
$ arc diff --only HEAD^
You'll be prompted to convert the file to binary:
Do you want to mark this file as binary and continue? [Y/n] y
Before this patch, that would be followed by:
Uploading 0 files...
...which is incorrect; we need to upload the new data. After this patch, this shows:
Uploading 1 files...
...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.
Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6815
2013-08-27 18:34:30 +02:00
|
|
|
$change->convertToBinaryChange($repository_api);
|
2011-05-21 16:16:12 +02:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2011-01-11 22:02:38 +01:00
|
|
|
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
$this->uploadFilesForChanges($changes);
|
2013-02-07 05:24:31 +01:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return $changes;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function getGitParentLogInfo() {
|
|
|
|
$info = array(
|
|
|
|
'parent' => null,
|
|
|
|
'base_revision' => null,
|
|
|
|
'base_path' => null,
|
2011-04-06 05:12:37 +02:00
|
|
|
'uuid' => null,
|
2011-01-10 00:22:25 +01:00
|
|
|
);
|
|
|
|
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
2012-07-18 01:16:18 +02:00
|
|
|
$parser = $this->newDiffParser();
|
2011-01-10 00:22:25 +01:00
|
|
|
$history_messages = $repository_api->getGitHistoryLog();
|
|
|
|
if (!$history_messages) {
|
|
|
|
// This can occur on the initial commit.
|
|
|
|
return $info;
|
|
|
|
}
|
|
|
|
$history_messages = $parser->parseDiff($history_messages);
|
|
|
|
|
|
|
|
foreach ($history_messages as $key => $change) {
|
|
|
|
try {
|
|
|
|
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
|
|
|
|
$change->getMetadata('message'));
|
|
|
|
if ($message->getRevisionID() && $info['parent'] === null) {
|
|
|
|
$info['parent'] = $message->getRevisionID();
|
|
|
|
}
|
|
|
|
if ($message->getGitSVNBaseRevision() &&
|
|
|
|
$info['base_revision'] === null) {
|
|
|
|
$info['base_revision'] = $message->getGitSVNBaseRevision();
|
|
|
|
$info['base_path'] = $message->getGitSVNBasePath();
|
|
|
|
}
|
2011-04-06 05:12:37 +02:00
|
|
|
if ($message->getGitSVNUUID()) {
|
|
|
|
$info['uuid'] = $message->getGitSVNUUID();
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
if ($info['parent'] && $info['base_revision']) {
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
} catch (ArcanistDifferentialCommitMessageParserException $ex) {
|
|
|
|
// Ignore.
|
2012-05-09 01:31:26 +02:00
|
|
|
} catch (ArcanistUsageException $ex) {
|
|
|
|
// Ignore an invalid Differential Revision field in the parent commit
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $info;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function primeSubversionWorkingCopyData($paths) {
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
|
|
|
$futures = array();
|
|
|
|
$targets = array();
|
|
|
|
foreach ($paths as $path => $mask) {
|
|
|
|
$futures[] = $repository_api->buildDiffFuture($path);
|
|
|
|
$targets[] = array('command' => 'diff', 'path' => $path);
|
|
|
|
$futures[] = $repository_api->buildInfoFuture($path);
|
|
|
|
$targets[] = array('command' => 'info', 'path' => $path);
|
|
|
|
}
|
|
|
|
|
2014-12-30 13:14:32 +01:00
|
|
|
$futures = id(new FutureIterator($futures))
|
|
|
|
->limit(8);
|
|
|
|
foreach ($futures as $key => $future) {
|
2011-01-10 00:22:25 +01:00
|
|
|
$target = $targets[$key];
|
|
|
|
if ($target['command'] == 'diff') {
|
|
|
|
$repository_api->primeSVNDiffResult(
|
|
|
|
$target['path'],
|
|
|
|
$future->resolve());
|
|
|
|
} else {
|
|
|
|
$repository_api->primeSVNInfoResult(
|
|
|
|
$target['path'],
|
|
|
|
$future->resolve());
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2012-02-01 23:33:01 +01:00
|
|
|
private function shouldAmend() {
|
2014-05-17 22:40:05 +02:00
|
|
|
if ($this->isRawDiffSource()) {
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2014-05-17 22:40:05 +02:00
|
|
|
if ($this->haveUncommittedChanges) {
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->getArgument('no-amend')) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
Dirty cache of HEAD commit after an amend/range reload
Summary:
Introducing `--head` caused us to run `git diff base..head` explicitly.
However, we can now hit this workflow:
- We resolve `HEAD` as commit `aaaa1`.
- This is cached.
- We notice dirty working copy changes and prompt the user to amend them to HEAD.
- The user accepts the amend.
- We amend, creating commit `bbbb2`.
- We dirty the commit range and reload the working copy. This //does not// dirty the cache of HEAD.
- We run `git diff`, but it uses the old cached HEAD: `git diff base..aaaa1`.
- This works fine (`aaaa1` still exists, it's just not on any branch) but produces the wrong diff (without amended changes).
To resolve this, implement the "dirty the cache when the range reloads" hook.
Also never try to amend if the user provides `--head`.
Test Plan:
Ran `arc diff --only --trace` in a working copy with a new commit and some uncommitted changes.
- Prior to this change, saw a `git diff base..aaaa1` command and the wrong diff.
- After this change, saw a `git diff base..bbbb2` command and the correct diff.
Reviewers: chad, csilvers, talshiri
Reviewed By: talshiri
Subscribers: epriestley, spicyj
Differential Revision: https://secure.phabricator.com/D9506
2014-06-13 00:46:15 +02:00
|
|
|
if ($this->getArgument('head') !== null) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2014-05-17 22:40:05 +02:00
|
|
|
// Run this last: with --raw or --raw-command, we won't have a repository
|
|
|
|
// API.
|
|
|
|
if ($this->isHistoryImmutable()) {
|
Fix some arc/mercurial issues
Summary:
- In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
- When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
- If you have uncommitted working copy changes, never try to amend.
Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.
Reviewers: dschleimer, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1507
Differential Revision: https://secure.phabricator.com/D2980
2012-07-18 01:16:11 +02:00
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
return true;
|
2012-02-01 23:33:01 +01:00
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
/* -( Lint and Unit Tests )------------------------------------------------ */
|
2011-01-10 00:22:25 +01:00
|
|
|
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
/**
|
|
|
|
* @task lintunit
|
|
|
|
*/
|
2012-08-06 22:40:20 +02:00
|
|
|
private function runLintUnit() {
|
|
|
|
$lint_result = $this->runLint();
|
|
|
|
$unit_result = $this->runUnit();
|
|
|
|
return array(
|
|
|
|
'lintResult' => $lint_result,
|
|
|
|
'unresolvedLint' => $this->unresolvedLint,
|
|
|
|
'postponedLinters' => $this->postponedLinters,
|
|
|
|
'unitResult' => $unit_result,
|
|
|
|
'testResults' => $this->testResults,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task lintunit
|
|
|
|
*/
|
|
|
|
private function runLint() {
|
2011-01-10 00:22:25 +01:00
|
|
|
if ($this->getArgument('nolint') ||
|
2012-01-05 22:36:24 +01:00
|
|
|
$this->getArgument('only') ||
|
2014-06-11 23:37:01 +02:00
|
|
|
$this->isRawDiffSource() ||
|
|
|
|
$this->getArgument('head')) {
|
2011-01-10 00:22:25 +01:00
|
|
|
return ArcanistLintWorkflow::RESULT_SKIP;
|
|
|
|
}
|
|
|
|
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut("Linting...\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
try {
|
2011-02-02 05:32:10 +01:00
|
|
|
$argv = $this->getPassthruArgumentsAsArgv('lint');
|
2012-12-17 21:54:08 +01:00
|
|
|
if ($repository_api->supportsCommitRanges()) {
|
Unify arguments for 'arc lint', 'arc unit'
Summary: See T645. These commands take inconsistent and overly-magical arguments
right now. Instead, make them behave consistently and allow them both to operate
on "arc <workflow> path path2 path3 ...", which is a generally useful workflow.
Test Plan: Ran "arc lint <path>", "arc unit <path>", "arc lint --rev
HEAD^^^^^^", "arc unit --rev HEAD^^^^^^^^^^^^", etc. Ran "arc diff --trace" and
verified --rev argument to child workflows.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley, btrahan
Maniphest Tasks: T645
Differential Revision: https://secure.phabricator.com/D1348
2012-01-09 21:40:50 +01:00
|
|
|
$argv[] = '--rev';
|
2012-12-17 21:54:08 +01:00
|
|
|
$argv[] = $repository_api->getBaseCommit();
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
Improve handling of `--background` with `--base`
Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.
Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.
Test Plan:
- Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
- Created a commit on master.
- Ran `arc diff HEAD^`.
- Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
- After this change, the command succeeds.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3574
2012-10-02 20:01:49 +02:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
$lint_workflow = $this->buildChildWorkflow('lint', $argv);
|
2011-01-30 02:18:32 +01:00
|
|
|
|
2012-02-04 00:26:13 +01:00
|
|
|
if ($this->shouldAmend()) {
|
2011-08-26 02:53:59 +02:00
|
|
|
// TODO: We should offer to create a checkpoint commit.
|
|
|
|
$lint_workflow->setShouldAmendChanges(true);
|
|
|
|
}
|
2011-01-30 02:18:32 +01:00
|
|
|
|
|
|
|
$lint_result = $lint_workflow->run();
|
2011-01-10 00:22:25 +01:00
|
|
|
|
|
|
|
switch ($lint_result) {
|
|
|
|
case ArcanistLintWorkflow::RESULT_OKAY:
|
2012-11-19 22:20:44 +01:00
|
|
|
if ($this->getArgument('advice') &&
|
|
|
|
$lint_workflow->getUnresolvedMessages()) {
|
|
|
|
$this->getErrorExcuse(
|
|
|
|
'lint',
|
2014-05-23 22:53:05 +02:00
|
|
|
'Lint issued unresolved advice.',
|
2012-11-19 22:20:44 +01:00
|
|
|
'lint-excuses');
|
|
|
|
} else {
|
|
|
|
$this->console->writeOut(
|
|
|
|
"<bg:green>** LINT OKAY **</bg> No lint problems.\n");
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
break;
|
|
|
|
case ArcanistLintWorkflow::RESULT_WARNINGS:
|
2012-10-22 23:46:31 +02:00
|
|
|
$this->getErrorExcuse(
|
|
|
|
'lint',
|
2014-05-23 22:53:05 +02:00
|
|
|
'Lint issued unresolved warnings.',
|
2012-06-20 03:14:26 +02:00
|
|
|
'lint-excuses');
|
2011-01-10 00:22:25 +01:00
|
|
|
break;
|
|
|
|
case ArcanistLintWorkflow::RESULT_ERRORS:
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut(
|
2011-01-10 00:22:25 +01:00
|
|
|
"<bg:red>** LINT ERRORS **</bg> Lint raised errors!\n");
|
2012-10-22 23:46:31 +02:00
|
|
|
$this->getErrorExcuse(
|
|
|
|
'lint',
|
2014-05-23 22:53:05 +02:00
|
|
|
'Lint issued unresolved errors!',
|
2012-06-20 03:14:26 +02:00
|
|
|
'lint-excuses');
|
2011-01-10 00:22:25 +01:00
|
|
|
break;
|
2012-07-03 00:53:22 +02:00
|
|
|
case ArcanistLintWorkflow::RESULT_POSTPONED:
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut(
|
2012-07-03 00:53:22 +02:00
|
|
|
"<bg:yellow>** LINT POSTPONED **</bg> ".
|
|
|
|
"Lint results are postponed.\n");
|
|
|
|
break;
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->unresolvedLint = array();
|
|
|
|
foreach ($lint_workflow->getUnresolvedMessages() as $message) {
|
2012-11-06 05:23:38 +01:00
|
|
|
$this->unresolvedLint[] = $message->toDictionary();
|
2012-08-06 22:40:20 +02:00
|
|
|
}
|
|
|
|
|
2012-07-03 00:53:22 +02:00
|
|
|
$this->postponedLinters = $lint_workflow->getPostponedLinters();
|
2011-01-12 07:13:31 +01:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return $lint_result;
|
|
|
|
} catch (ArcanistNoEngineException $ex) {
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut("No lint engine configured for this project.\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
} catch (ArcanistNoEffectException $ex) {
|
2013-01-24 23:45:21 +01:00
|
|
|
$this->console->writeOut($ex->getMessage()."\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
/**
|
|
|
|
* @task lintunit
|
|
|
|
*/
|
2012-08-06 22:40:20 +02:00
|
|
|
private function runUnit() {
|
2011-01-10 00:22:25 +01:00
|
|
|
if ($this->getArgument('nounit') ||
|
2012-01-05 22:36:24 +01:00
|
|
|
$this->getArgument('only') ||
|
2014-06-11 23:37:01 +02:00
|
|
|
$this->isRawDiffSource() ||
|
|
|
|
$this->getArgument('head')) {
|
2011-01-10 00:22:25 +01:00
|
|
|
return ArcanistUnitWorkflow::RESULT_SKIP;
|
|
|
|
}
|
|
|
|
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut("Running unit tests...\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
try {
|
2011-02-02 05:32:10 +01:00
|
|
|
$argv = $this->getPassthruArgumentsAsArgv('unit');
|
2012-12-17 21:54:08 +01:00
|
|
|
if ($repository_api->supportsCommitRanges()) {
|
Unify arguments for 'arc lint', 'arc unit'
Summary: See T645. These commands take inconsistent and overly-magical arguments
right now. Instead, make them behave consistently and allow them both to operate
on "arc <workflow> path path2 path3 ...", which is a generally useful workflow.
Test Plan: Ran "arc lint <path>", "arc unit <path>", "arc lint --rev
HEAD^^^^^^", "arc unit --rev HEAD^^^^^^^^^^^^", etc. Ran "arc diff --trace" and
verified --rev argument to child workflows.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley, btrahan
Maniphest Tasks: T645
Differential Revision: https://secure.phabricator.com/D1348
2012-01-09 21:40:50 +01:00
|
|
|
$argv[] = '--rev';
|
2012-12-17 21:54:08 +01:00
|
|
|
$argv[] = $repository_api->getBaseCommit();
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
2012-08-07 01:56:11 +02:00
|
|
|
$unit_workflow = $this->buildChildWorkflow('unit', $argv);
|
|
|
|
$unit_result = $unit_workflow->run();
|
2012-06-01 07:57:29 +02:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
switch ($unit_result) {
|
|
|
|
case ArcanistUnitWorkflow::RESULT_OKAY:
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut(
|
2011-01-10 00:22:25 +01:00
|
|
|
"<bg:green>** UNIT OKAY **</bg> No unit test failures.\n");
|
|
|
|
break;
|
|
|
|
case ArcanistUnitWorkflow::RESULT_UNSOUND:
|
2012-07-17 00:54:11 +02:00
|
|
|
if ($this->getArgument('ignore-unsound-tests')) {
|
|
|
|
echo phutil_console_format(
|
|
|
|
"<bg:yellow>** UNIT UNSOUND **</bg> Unit testing raised errors, ".
|
|
|
|
"but all failing tests are unsound.\n");
|
|
|
|
} else {
|
2012-08-16 00:54:34 +02:00
|
|
|
$continue = $this->console->confirm(
|
2014-05-23 22:53:05 +02:00
|
|
|
'Unit test results included failures, but all failing tests '.
|
|
|
|
'are known to be unsound. Ignore unsound test failures?');
|
2012-07-17 00:54:11 +02:00
|
|
|
if (!$continue) {
|
|
|
|
throw new ArcanistUserAbortException();
|
|
|
|
}
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
break;
|
|
|
|
case ArcanistUnitWorkflow::RESULT_FAIL:
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut(
|
2011-01-10 00:22:25 +01:00
|
|
|
"<bg:red>** UNIT ERRORS **</bg> Unit testing raised errors!\n");
|
2012-10-22 23:46:31 +02:00
|
|
|
$this->getErrorExcuse(
|
|
|
|
'unit',
|
2014-05-23 22:53:05 +02:00
|
|
|
'Unit test results include failures!',
|
2012-06-20 03:14:26 +02:00
|
|
|
'unit-excuses');
|
2011-01-10 00:22:25 +01:00
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->testResults = array();
|
|
|
|
foreach ($unit_workflow->getTestResults() as $test) {
|
2015-01-04 20:48:32 +01:00
|
|
|
$this->testResults[] = $test->toDictionary();
|
2012-08-06 22:40:20 +02:00
|
|
|
}
|
2011-01-12 07:13:31 +01:00
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
return $unit_result;
|
|
|
|
} catch (ArcanistNoEngineException $ex) {
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut(
|
|
|
|
"No unit test engine is configured for this project.\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
} catch (ArcanistNoEffectException $ex) {
|
2013-01-24 23:45:21 +01:00
|
|
|
$this->console->writeOut($ex->getMessage()."\n");
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-10-13 00:21:00 +02:00
|
|
|
public function getTestResults() {
|
|
|
|
return $this->testResults;
|
|
|
|
}
|
|
|
|
|
2012-11-02 06:41:00 +01:00
|
|
|
private function getSkipExcuse($prompt, $history) {
|
|
|
|
$excuse = $this->getArgument('excuse');
|
|
|
|
|
|
|
|
if ($excuse === null) {
|
|
|
|
$history = $this->getRepositoryAPI()->getScratchFilePath($history);
|
|
|
|
$excuse = phutil_console_prompt($prompt, $history);
|
|
|
|
if ($excuse == '') {
|
|
|
|
throw new ArcanistUserAbortException();
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $excuse;
|
|
|
|
}
|
|
|
|
|
2012-10-22 23:46:31 +02:00
|
|
|
private function getErrorExcuse($type, $prompt, $history) {
|
2012-06-01 07:57:29 +02:00
|
|
|
if ($this->getArgument('excuse')) {
|
2012-10-24 00:21:01 +02:00
|
|
|
$this->console->sendMessage(array(
|
|
|
|
'type' => $type,
|
2014-05-23 22:53:05 +02:00
|
|
|
'confirm' => $prompt.' Ignore them?',
|
2012-10-24 00:21:01 +02:00
|
|
|
));
|
2012-10-22 23:46:31 +02:00
|
|
|
return;
|
2012-06-01 07:57:29 +02:00
|
|
|
}
|
2012-02-23 00:57:55 +01:00
|
|
|
|
2012-06-20 03:14:26 +02:00
|
|
|
$history = $this->getRepositoryAPI()->getScratchFilePath($history);
|
|
|
|
|
2014-05-23 22:53:05 +02:00
|
|
|
$prompt .= ' Provide explanation to continue or press Enter to abort.';
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->console->writeOut("\n\n%s", phutil_console_wrap($prompt));
|
2012-10-22 23:46:31 +02:00
|
|
|
$this->console->sendMessage(array(
|
|
|
|
'type' => $type,
|
2014-05-23 22:53:05 +02:00
|
|
|
'prompt' => 'Explanation:',
|
2012-10-22 23:46:31 +02:00
|
|
|
'history' => $history,
|
|
|
|
));
|
|
|
|
}
|
|
|
|
|
|
|
|
public function handleServerMessage(PhutilConsoleMessage $message) {
|
|
|
|
$data = $message->getData();
|
2013-08-26 14:41:32 +02:00
|
|
|
|
|
|
|
if ($this->getArgument('excuse')) {
|
|
|
|
try {
|
|
|
|
phutil_console_require_tty();
|
|
|
|
} catch (PhutilConsoleStdinNotInteractiveException $ex) {
|
|
|
|
$this->excuses[$data['type']] = $this->getArgument('excuse');
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2012-10-24 00:21:01 +02:00
|
|
|
$response = '';
|
|
|
|
if (isset($data['prompt'])) {
|
|
|
|
$response = phutil_console_prompt($data['prompt'], idx($data, 'history'));
|
|
|
|
} else if (phutil_console_confirm($data['confirm'])) {
|
|
|
|
$response = $this->getArgument('excuse');
|
|
|
|
}
|
2012-10-22 23:46:31 +02:00
|
|
|
if ($response == '') {
|
2012-06-01 07:57:29 +02:00
|
|
|
throw new ArcanistUserAbortException();
|
2012-02-23 00:57:55 +01:00
|
|
|
}
|
2012-10-22 23:46:31 +02:00
|
|
|
$this->excuses[$data['type']] = $response;
|
|
|
|
return null;
|
2012-02-23 00:57:55 +01:00
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
/* -( Commit and Update Messages )----------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function buildCommitMessage() {
|
2012-04-12 19:32:54 +02:00
|
|
|
if ($this->getArgument('preview') || $this->getArgument('only')) {
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$is_create = $this->getArgument('create');
|
2012-01-09 18:33:55 +01:00
|
|
|
$is_update = $this->getArgument('update');
|
2012-01-05 22:36:24 +01:00
|
|
|
$is_raw = $this->isRawDiffSource();
|
2012-01-13 04:03:11 +01:00
|
|
|
$is_message = $this->getArgument('use-commit-message');
|
With `--verbatim`, update some fields automatically when updating revisions
Summary:
Essentially D2391, but with, uh, more comments?
- I forgot that we already implemented shouldOverwriteWhenCommitMessageIsEdited(). This patch already behaves nearly correctly.
- Requires changes in D2412.
- Use `'edit' => 'edit'`, which does the same thing as `'edit' => true`, but is more correct after the "edit" / "create" split.
- Under "--verbatim", always get the message "from the user", which means "from the working copy" because verbtatim disables the editor part.
Test Plan:
- Created and updated revisions with `arc diff`.
- Created and updated revisions with `arc diff --verbatim`.
- Updated revisions with `arc diff --edit`.
Reviewers: jungejason, btrahan
Reviewed By: jungejason
CC: vrana, aran
Differential Revision: https://secure.phabricator.com/D2411
2012-05-07 17:16:29 +02:00
|
|
|
$is_verbatim = $this->getArgument('verbatim');
|
2012-01-13 04:03:11 +01:00
|
|
|
|
|
|
|
if ($is_message) {
|
|
|
|
return $this->getCommitMessageFromCommit($is_message);
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
|
With `--verbatim`, update some fields automatically when updating revisions
Summary:
Essentially D2391, but with, uh, more comments?
- I forgot that we already implemented shouldOverwriteWhenCommitMessageIsEdited(). This patch already behaves nearly correctly.
- Requires changes in D2412.
- Use `'edit' => 'edit'`, which does the same thing as `'edit' => true`, but is more correct after the "edit" / "create" split.
- Under "--verbatim", always get the message "from the user", which means "from the working copy" because verbtatim disables the editor part.
Test Plan:
- Created and updated revisions with `arc diff`.
- Created and updated revisions with `arc diff --verbatim`.
- Updated revisions with `arc diff --edit`.
Reviewers: jungejason, btrahan
Reviewed By: jungejason
CC: vrana, aran
Differential Revision: https://secure.phabricator.com/D2411
2012-05-07 17:16:29 +02:00
|
|
|
if ($is_verbatim) {
|
|
|
|
return $this->getCommitMessageFromUser();
|
|
|
|
}
|
|
|
|
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
|
Make "--auto" the default for "arc diff"
Summary:
NOTE: This is a disruptive change to how 'arc diff' behaves by default.
Many years ago, Differential (then DiffCamp) supported SVN. Someone added a "-i" mode to the "diffcamp" script so you could "git show | diffcamp -i", and thus we were
forever bound to storing metadata in commit messages.
But this isn't a common use case outside of Facebook + git-svn, and isn't very flexible. We now have a great deal of flexibility to identify revisions based on
hashes, branch names, etc, and to sync metdata from web to CLI and back. I want to jettison the commit-message-bound artifacts of the tool's history, and move to a
more flexible, modern workflow.
I added "--auto" a while ago, which figures out if you want to create or update a diff automatically, and then prompts you for whatever data it needs, reading
information if it can from commit messages in the range. This is a vastly better workflow in general, especially for SVN and Mercurial users (who currently need to
jump to the web UI to create revisions). It's better for git users too, since they don't need to use template commits and don't have to muck with or configure
templates. However, it's a nontrivial change to git users' core workflow that is clearly different in more ways than it is clearly better.
- This might be contentious, but probably not toooo much, I hope?
- I also deleted the (fairly ridiculous) workflow where we sync commit message changes from the working copy. I think two or three people will be REALLY upset about
this but I have no sympathy. "--edit" covers this and has been around for like two years now, and making the commit message and web dual-authoritative was always a bad
idea that we only ever half-accommodated anyway (see giant swaths of removed TOOD nonsense).
Test Plan:
- I've been using "--auto" exclusively for like a month, it seems to work well.
- Created/updated SVN, Git and HG diffs with "arc diff" under this workflow.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: Makinde, vrana, zeeg, mbautin, aran, yairlivne, 20after4
Maniphest Tasks: T614
Differential Revision: https://secure.phabricator.com/D2080
2012-04-07 19:39:27 +02:00
|
|
|
if (!$is_raw && !$is_create && !$is_update) {
|
2012-02-20 21:51:01 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
$revisions = $repository_api->loadWorkingCopyDifferentialRevisions(
|
|
|
|
$this->getConduit(),
|
|
|
|
array(
|
|
|
|
'authors' => array($this->getUserPHID()),
|
2012-02-22 19:07:43 +01:00
|
|
|
'status' => 'status-open',
|
2012-02-20 21:51:01 +01:00
|
|
|
));
|
|
|
|
if (!$revisions) {
|
|
|
|
$is_create = true;
|
|
|
|
} else if (count($revisions) == 1) {
|
|
|
|
$revision = head($revisions);
|
|
|
|
$is_update = $revision['id'];
|
|
|
|
} else {
|
|
|
|
throw new ArcanistUsageException(
|
2012-04-10 21:06:41 +02:00
|
|
|
"There are several revisions which match the working copy:\n\n".
|
2012-02-20 21:51:01 +01:00
|
|
|
$this->renderRevisionList($revisions)."\n".
|
|
|
|
"Use '--update' to choose one, or '--create' to create a new ".
|
|
|
|
"revision.");
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$message = null;
|
|
|
|
if ($is_create) {
|
|
|
|
$message_file = $this->getArgument('message-file');
|
|
|
|
if ($message_file) {
|
|
|
|
return $this->getCommitMessageFromFile($message_file);
|
|
|
|
} else {
|
|
|
|
return $this->getCommitMessageFromUser();
|
|
|
|
}
|
Make "--auto" the default for "arc diff"
Summary:
NOTE: This is a disruptive change to how 'arc diff' behaves by default.
Many years ago, Differential (then DiffCamp) supported SVN. Someone added a "-i" mode to the "diffcamp" script so you could "git show | diffcamp -i", and thus we were
forever bound to storing metadata in commit messages.
But this isn't a common use case outside of Facebook + git-svn, and isn't very flexible. We now have a great deal of flexibility to identify revisions based on
hashes, branch names, etc, and to sync metdata from web to CLI and back. I want to jettison the commit-message-bound artifacts of the tool's history, and move to a
more flexible, modern workflow.
I added "--auto" a while ago, which figures out if you want to create or update a diff automatically, and then prompts you for whatever data it needs, reading
information if it can from commit messages in the range. This is a vastly better workflow in general, especially for SVN and Mercurial users (who currently need to
jump to the web UI to create revisions). It's better for git users too, since they don't need to use template commits and don't have to muck with or configure
templates. However, it's a nontrivial change to git users' core workflow that is clearly different in more ways than it is clearly better.
- This might be contentious, but probably not toooo much, I hope?
- I also deleted the (fairly ridiculous) workflow where we sync commit message changes from the working copy. I think two or three people will be REALLY upset about
this but I have no sympathy. "--edit" covers this and has been around for like two years now, and making the commit message and web dual-authoritative was always a bad
idea that we only ever half-accommodated anyway (see giant swaths of removed TOOD nonsense).
Test Plan:
- I've been using "--auto" exclusively for like a month, it seems to work well.
- Created/updated SVN, Git and HG diffs with "arc diff" under this workflow.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: Makinde, vrana, zeeg, mbautin, aran, yairlivne, 20after4
Maniphest Tasks: T614
Differential Revision: https://secure.phabricator.com/D2080
2012-04-07 19:39:27 +02:00
|
|
|
} else if ($is_update) {
|
2012-05-12 03:40:02 +02:00
|
|
|
$revision_id = $this->normalizeRevisionID($is_update);
|
|
|
|
if (!is_numeric($revision_id)) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
'Parameter to --update must be a Differential Revision number');
|
|
|
|
}
|
|
|
|
return $this->getCommitMessageFromRevision($revision_id);
|
Make "--auto" the default for "arc diff"
Summary:
NOTE: This is a disruptive change to how 'arc diff' behaves by default.
Many years ago, Differential (then DiffCamp) supported SVN. Someone added a "-i" mode to the "diffcamp" script so you could "git show | diffcamp -i", and thus we were
forever bound to storing metadata in commit messages.
But this isn't a common use case outside of Facebook + git-svn, and isn't very flexible. We now have a great deal of flexibility to identify revisions based on
hashes, branch names, etc, and to sync metdata from web to CLI and back. I want to jettison the commit-message-bound artifacts of the tool's history, and move to a
more flexible, modern workflow.
I added "--auto" a while ago, which figures out if you want to create or update a diff automatically, and then prompts you for whatever data it needs, reading
information if it can from commit messages in the range. This is a vastly better workflow in general, especially for SVN and Mercurial users (who currently need to
jump to the web UI to create revisions). It's better for git users too, since they don't need to use template commits and don't have to muck with or configure
templates. However, it's a nontrivial change to git users' core workflow that is clearly different in more ways than it is clearly better.
- This might be contentious, but probably not toooo much, I hope?
- I also deleted the (fairly ridiculous) workflow where we sync commit message changes from the working copy. I think two or three people will be REALLY upset about
this but I have no sympathy. "--edit" covers this and has been around for like two years now, and making the commit message and web dual-authoritative was always a bad
idea that we only ever half-accommodated anyway (see giant swaths of removed TOOD nonsense).
Test Plan:
- I've been using "--auto" exclusively for like a month, it seems to work well.
- Created/updated SVN, Git and HG diffs with "arc diff" under this workflow.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: Makinde, vrana, zeeg, mbautin, aran, yairlivne, 20after4
Maniphest Tasks: T614
Differential Revision: https://secure.phabricator.com/D2080
2012-04-07 19:39:27 +02:00
|
|
|
} else {
|
|
|
|
// This is --raw without enough info to create a revision, so force just
|
|
|
|
// a diff.
|
2012-01-05 22:36:24 +01:00
|
|
|
return null;
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-01-13 04:03:11 +01:00
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
2012-07-01 20:06:05 +02:00
|
|
|
private function getCommitMessageFromCommit($commit) {
|
|
|
|
$text = $this->getRepositoryAPI()->getCommitMessage($commit);
|
|
|
|
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
|
2012-01-13 04:03:11 +01:00
|
|
|
$message->pullDataFromConduit($this->getConduit());
|
|
|
|
$this->validateCommitMessage($message);
|
|
|
|
return $message;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
2011-12-03 00:50:36 +01:00
|
|
|
private function getCommitMessageFromUser() {
|
|
|
|
$conduit = $this->getConduit();
|
|
|
|
|
2012-02-21 21:35:39 +01:00
|
|
|
$template = null;
|
2011-12-03 00:50:36 +01:00
|
|
|
|
2012-05-04 09:08:42 +02:00
|
|
|
if (!$this->getArgument('verbatim')) {
|
|
|
|
$saved = $this->readScratchFile('create-message');
|
|
|
|
if ($saved) {
|
|
|
|
$where = $this->getReadableScratchFilePath('create-message');
|
|
|
|
|
|
|
|
$preview = explode("\n", $saved);
|
|
|
|
$preview = array_shift($preview);
|
|
|
|
$preview = trim($preview);
|
2014-08-30 00:15:18 +02:00
|
|
|
$preview = id(new PhutilUTF8StringTruncator())
|
|
|
|
->setMaximumGlyphs(64)
|
|
|
|
->truncateString($preview);
|
2012-05-04 09:08:42 +02:00
|
|
|
|
|
|
|
if ($preview) {
|
|
|
|
$preview = "Message begins:\n\n {$preview}\n\n";
|
|
|
|
} else {
|
|
|
|
$preview = null;
|
|
|
|
}
|
2012-02-21 21:35:39 +01:00
|
|
|
|
2012-05-04 09:08:42 +02:00
|
|
|
echo
|
|
|
|
"You have a saved revision message in '{$where}'.\n".
|
|
|
|
"{$preview}".
|
|
|
|
"You can use this message, or discard it.";
|
2012-02-21 21:35:39 +01:00
|
|
|
|
2012-05-04 09:08:42 +02:00
|
|
|
$use = phutil_console_confirm(
|
2014-05-23 22:53:05 +02:00
|
|
|
'Do you want to use this message?',
|
2012-05-04 09:08:42 +02:00
|
|
|
$default_no = false);
|
|
|
|
if ($use) {
|
|
|
|
$template = $saved;
|
|
|
|
} else {
|
|
|
|
$this->removeScratchFile('create-message');
|
|
|
|
}
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$template_is_default = false;
|
2012-02-29 02:00:35 +01:00
|
|
|
$notes = array();
|
2012-05-07 22:18:16 +02:00
|
|
|
$included = array();
|
2012-02-21 21:35:39 +01:00
|
|
|
|
2012-09-28 20:49:04 +02:00
|
|
|
list($fields, $notes, $included_commits) = $this->getDefaultCreateFields();
|
2012-05-07 22:18:16 +02:00
|
|
|
if ($template) {
|
|
|
|
$fields = array();
|
|
|
|
$notes = array();
|
|
|
|
} else {
|
2012-02-29 02:00:35 +01:00
|
|
|
if (!$fields) {
|
|
|
|
$template_is_default = true;
|
|
|
|
}
|
|
|
|
|
2012-06-12 22:29:05 +02:00
|
|
|
if ($notes) {
|
|
|
|
$commit = head($this->getRepositoryAPI()->getLocalCommitInformation());
|
|
|
|
$template = $commit['message'];
|
|
|
|
} else {
|
|
|
|
$template = $conduit->callMethodSynchronous(
|
|
|
|
'differential.getcommitmessage',
|
|
|
|
array(
|
|
|
|
'revision_id' => null,
|
|
|
|
'edit' => 'create',
|
|
|
|
'fields' => $fields,
|
|
|
|
));
|
|
|
|
}
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
2014-03-15 19:25:49 +01:00
|
|
|
|
2012-12-20 21:11:57 +01:00
|
|
|
$old_message = $template;
|
2012-02-21 21:35:39 +01:00
|
|
|
|
2012-09-28 20:49:04 +02:00
|
|
|
$included = array();
|
|
|
|
if ($included_commits) {
|
|
|
|
foreach ($included_commits as $commit) {
|
|
|
|
$included[] = ' '.$commit;
|
2012-05-07 22:18:16 +02:00
|
|
|
}
|
2012-05-25 02:48:56 +02:00
|
|
|
$in_branch = '';
|
|
|
|
if (!$this->isRawDiffSource()) {
|
|
|
|
$in_branch = ' in branch '.$this->getRepositoryAPI()->getBranchName();
|
|
|
|
}
|
2012-05-07 22:18:16 +02:00
|
|
|
$included = array_merge(
|
|
|
|
array(
|
2014-05-23 22:53:05 +02:00
|
|
|
'',
|
2012-05-25 02:48:56 +02:00
|
|
|
"Included commits{$in_branch}:",
|
2014-05-23 22:53:05 +02:00
|
|
|
'',
|
2012-05-07 22:18:16 +02:00
|
|
|
),
|
2012-09-28 20:49:04 +02:00
|
|
|
$included);
|
2012-05-07 22:18:16 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
$issues = array_merge(
|
|
|
|
array(
|
|
|
|
'NEW DIFFERENTIAL REVISION',
|
|
|
|
'Describe the changes in this new revision.',
|
|
|
|
),
|
|
|
|
$included,
|
|
|
|
array(
|
2012-09-28 20:49:04 +02:00
|
|
|
'',
|
2012-05-07 22:18:16 +02:00
|
|
|
'arc could not identify any existing revision in your working copy.',
|
|
|
|
'If you intended to update an existing revision, use:',
|
|
|
|
'',
|
|
|
|
' $ arc diff --update <revision>',
|
|
|
|
));
|
2012-02-29 02:00:35 +01:00
|
|
|
if ($notes) {
|
|
|
|
$issues = array_merge($issues, array(''), $notes);
|
|
|
|
}
|
2012-02-21 21:35:39 +01:00
|
|
|
|
|
|
|
$done = false;
|
2012-04-10 21:06:41 +02:00
|
|
|
$first = true;
|
2012-02-21 21:35:39 +01:00
|
|
|
while (!$done) {
|
2012-05-24 20:48:59 +02:00
|
|
|
$template = rtrim($template, "\r\n")."\n\n";
|
2012-02-21 21:35:39 +01:00
|
|
|
foreach ($issues as $issue) {
|
2015-04-12 23:21:07 +02:00
|
|
|
$template .= rtrim('# '.$issue)."\n";
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
$template .= "\n";
|
|
|
|
|
2012-04-10 21:06:41 +02:00
|
|
|
if ($first && $this->getArgument('verbatim') && !$template_is_default) {
|
|
|
|
$new_template = $template;
|
|
|
|
} else {
|
2012-07-12 01:55:11 +02:00
|
|
|
$new_template = $this->newInteractiveEditor($template)
|
2012-04-10 21:06:41 +02:00
|
|
|
->setName('new-commit')
|
|
|
|
->editInteractively();
|
|
|
|
}
|
|
|
|
$first = false;
|
2012-02-21 21:35:39 +01:00
|
|
|
|
|
|
|
if ($template_is_default && ($new_template == $template)) {
|
2014-05-23 22:53:05 +02:00
|
|
|
throw new ArcanistUsageException('Template not edited.');
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
|
2012-04-18 15:08:41 +02:00
|
|
|
$template = ArcanistCommentRemover::removeComments($new_template);
|
2012-09-28 20:49:04 +02:00
|
|
|
|
2014-05-17 22:40:05 +02:00
|
|
|
// With --raw-command, we may not have a repository API.
|
|
|
|
if ($this->hasRepositoryAPI()) {
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
// special check for whether to amend here. optimizes a common git
|
|
|
|
// workflow. we can't do this for mercurial because the mq extension
|
|
|
|
// is popular and incompatible with hg commit --amend ; see T2011.
|
|
|
|
$should_amend = (count($included_commits) == 1 &&
|
|
|
|
$repository_api instanceof ArcanistGitAPI &&
|
|
|
|
$this->shouldAmend());
|
|
|
|
} else {
|
|
|
|
$should_amend = false;
|
|
|
|
}
|
|
|
|
|
2012-11-02 00:47:08 +01:00
|
|
|
if ($should_amend) {
|
2012-12-20 21:11:57 +01:00
|
|
|
$wrote = (rtrim($old_message) != rtrim($template));
|
|
|
|
if ($wrote) {
|
|
|
|
$repository_api->amendCommit($template);
|
|
|
|
$where = 'commit message';
|
|
|
|
}
|
2012-09-28 20:49:04 +02:00
|
|
|
} else {
|
|
|
|
$wrote = $this->writeScratchFile('create-message', $template);
|
|
|
|
$where = "'".$this->getReadableScratchFilePath('create-message')."'";
|
|
|
|
}
|
2012-02-21 21:35:39 +01:00
|
|
|
|
|
|
|
try {
|
|
|
|
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
|
|
|
|
$template);
|
|
|
|
$message->pullDataFromConduit($conduit);
|
|
|
|
$this->validateCommitMessage($message);
|
|
|
|
$done = true;
|
|
|
|
} catch (ArcanistDifferentialCommitMessageParserException $ex) {
|
|
|
|
echo "Commit message has errors:\n\n";
|
|
|
|
$issues = array('Resolve these errors:');
|
|
|
|
foreach ($ex->getParserErrors() as $error) {
|
2012-06-12 21:10:51 +02:00
|
|
|
echo phutil_console_wrap("- ".$error."\n", 6);
|
2012-02-21 21:35:39 +01:00
|
|
|
$issues[] = ' - '.$error;
|
|
|
|
}
|
|
|
|
echo "\n";
|
2014-05-23 22:53:05 +02:00
|
|
|
echo 'You must resolve these errors to continue.';
|
2012-02-21 21:35:39 +01:00
|
|
|
$again = phutil_console_confirm(
|
2014-05-23 22:53:05 +02:00
|
|
|
'Do you want to edit the message?',
|
2012-02-21 21:35:39 +01:00
|
|
|
$default_no = false);
|
|
|
|
if ($again) {
|
|
|
|
// Keep going.
|
|
|
|
} else {
|
|
|
|
$saved = null;
|
|
|
|
if ($wrote) {
|
2012-09-28 20:49:04 +02:00
|
|
|
$saved = "A copy was saved to {$where}.";
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
throw new ArcanistUsageException(
|
2012-02-29 02:00:35 +01:00
|
|
|
"Message has unresolved errrors. {$saved}");
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
} catch (Exception $ex) {
|
|
|
|
if ($wrote) {
|
2012-09-28 20:49:04 +02:00
|
|
|
echo phutil_console_wrap("(Message saved to {$where}.)\n");
|
2012-02-21 21:35:39 +01:00
|
|
|
}
|
|
|
|
throw $ex;
|
|
|
|
}
|
2011-12-03 00:50:36 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return $message;
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
2011-12-03 00:50:36 +01:00
|
|
|
private function getCommitMessageFromFile($file) {
|
|
|
|
$conduit = $this->getConduit();
|
|
|
|
|
|
|
|
$data = Filesystem::readFile($file);
|
|
|
|
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus($data);
|
|
|
|
$message->pullDataFromConduit($conduit);
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
$this->validateCommitMessage($message);
|
|
|
|
|
2011-12-03 00:50:36 +01:00
|
|
|
return $message;
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
|
2012-01-09 18:33:55 +01:00
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function getCommitMessageFromRevision($revision_id) {
|
2012-05-12 03:40:02 +02:00
|
|
|
$id = $revision_id;
|
2012-01-09 18:33:55 +01:00
|
|
|
|
|
|
|
$revision = $this->getConduit()->callMethodSynchronous(
|
|
|
|
'differential.query',
|
|
|
|
array(
|
|
|
|
'ids' => array($id),
|
|
|
|
));
|
|
|
|
$revision = head($revision);
|
|
|
|
|
|
|
|
if (!$revision) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
"Revision '{$revision_id}' does not exist!");
|
|
|
|
}
|
|
|
|
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
$this->checkRevisionOwnership($revision);
|
2012-01-09 18:33:55 +01:00
|
|
|
|
|
|
|
$message = $this->getConduit()->callMethodSynchronous(
|
|
|
|
'differential.getcommitmessage',
|
|
|
|
array(
|
|
|
|
'revision_id' => $id,
|
|
|
|
'edit' => false,
|
|
|
|
));
|
2012-10-16 20:51:15 +02:00
|
|
|
$this->commitMessageFromRevision = $message;
|
2012-01-09 18:33:55 +01:00
|
|
|
|
|
|
|
$obj = ArcanistDifferentialCommitMessage::newFromRawCorpus($message);
|
|
|
|
$obj->pullDataFromConduit($this->getConduit());
|
|
|
|
|
|
|
|
return $obj;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function validateCommitMessage(
|
|
|
|
ArcanistDifferentialCommitMessage $message) {
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
$futures = array();
|
|
|
|
|
|
|
|
$revision_id = $message->getRevisionID();
|
|
|
|
if ($revision_id) {
|
|
|
|
$futures['revision'] = $this->getConduit()->callMethod(
|
|
|
|
'differential.query',
|
|
|
|
array(
|
|
|
|
'ids' => array($revision_id),
|
|
|
|
));
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$reviewers = $message->getFieldValue('reviewerPHIDs');
|
|
|
|
if (!$reviewers) {
|
2014-05-23 22:53:05 +02:00
|
|
|
$confirm = 'You have not specified any reviewers. Continue anyway?';
|
2012-01-05 20:26:17 +01:00
|
|
|
if (!phutil_console_confirm($confirm)) {
|
|
|
|
throw new ArcanistUsageException('Specify reviewers and retry.');
|
|
|
|
}
|
2012-05-18 04:27:24 +02:00
|
|
|
} else {
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
$futures['reviewers'] = $this->getConduit()->callMethod(
|
2012-05-18 19:27:44 +02:00
|
|
|
'user.query',
|
2012-05-18 04:27:24 +02:00
|
|
|
array(
|
2012-05-18 19:27:44 +02:00
|
|
|
'phids' => $reviewers,
|
2012-05-18 04:27:24 +02:00
|
|
|
));
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
}
|
|
|
|
|
2014-12-30 13:14:32 +01:00
|
|
|
foreach (new FutureIterator($futures) as $key => $future) {
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
$result = $future->resolve();
|
|
|
|
switch ($key) {
|
|
|
|
case 'revision':
|
|
|
|
if (empty($result)) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
"There is no revision D{$revision_id}.");
|
|
|
|
}
|
|
|
|
$this->checkRevisionOwnership(head($result));
|
|
|
|
break;
|
|
|
|
case 'reviewers':
|
|
|
|
$untils = array();
|
|
|
|
foreach ($result as $user) {
|
|
|
|
if (idx($user, 'currentStatus') == 'away') {
|
|
|
|
$untils[] = $user['currentStatusUntil'];
|
|
|
|
}
|
|
|
|
}
|
|
|
|
if (count($untils) == count($reviewers)) {
|
|
|
|
$until = date('l, M j Y', min($untils));
|
|
|
|
$confirm = "All reviewers are away until {$until}. ".
|
|
|
|
"Continue anyway?";
|
|
|
|
if (!phutil_console_confirm($confirm)) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
'Specify available reviewers and retry.');
|
|
|
|
}
|
|
|
|
}
|
|
|
|
break;
|
2012-05-18 04:27:24 +02:00
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task message
|
|
|
|
*/
|
2012-08-08 21:08:41 +02:00
|
|
|
private function getUpdateMessage(array $fields, $template = '') {
|
2012-03-20 03:15:58 +01:00
|
|
|
if ($this->getArgument('raw')) {
|
|
|
|
throw new ArcanistUsageException(
|
|
|
|
"When using '--raw' to update a revision, specify an update message ".
|
|
|
|
"with '--message'. (Normally, we'd launch an editor to ask you for a ".
|
2012-03-20 03:16:50 +01:00
|
|
|
"message, but can not do that because stdin is the diff source.)");
|
2012-03-20 03:15:58 +01:00
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
// When updating a revision using git without specifying '--message', try
|
|
|
|
// to prefill with the message in HEAD if it isn't a template message. The
|
|
|
|
// idea is that if you do:
|
|
|
|
//
|
|
|
|
// $ git commit -a -m 'fix some junk'
|
|
|
|
// $ arc diff
|
|
|
|
//
|
2012-02-23 18:18:49 +01:00
|
|
|
// ...you shouldn't have to retype the update message. Similar things apply
|
|
|
|
// to Mercurial.
|
|
|
|
|
2012-08-08 21:08:41 +02:00
|
|
|
if ($template == '') {
|
|
|
|
$comments = $this->getDefaultUpdateMessage();
|
|
|
|
|
|
|
|
$template =
|
|
|
|
rtrim($comments).
|
|
|
|
"\n\n".
|
|
|
|
"# Updating D{$fields['revisionID']}: {$fields['title']}\n".
|
|
|
|
"#\n".
|
|
|
|
"# Enter a brief description of the changes included in this update.\n".
|
|
|
|
"# The first line is used as subject, next lines as comment.\n".
|
|
|
|
"#\n".
|
|
|
|
"# If you intended to create a new revision, use:\n".
|
|
|
|
"# $ arc diff --create\n".
|
|
|
|
"\n";
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
|
2012-07-12 01:55:11 +02:00
|
|
|
$comments = $this->newInteractiveEditor($template)
|
2012-01-05 20:26:17 +01:00
|
|
|
->setName('differential-update-comments')
|
|
|
|
->editInteractively();
|
|
|
|
|
|
|
|
return $comments;
|
|
|
|
}
|
|
|
|
|
2012-02-29 02:00:35 +01:00
|
|
|
private function getDefaultCreateFields() {
|
2012-05-21 01:39:38 +02:00
|
|
|
$result = array(array(), array(), array());
|
2012-02-29 02:00:35 +01:00
|
|
|
|
2012-03-05 19:08:50 +01:00
|
|
|
if ($this->isRawDiffSource()) {
|
2012-05-21 01:39:38 +02:00
|
|
|
return $result;
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$local = $repository_api->getLocalCommitInformation();
|
|
|
|
if ($local) {
|
|
|
|
$result = $this->parseCommitMessagesIntoFields($local);
|
2013-02-22 21:33:31 +01:00
|
|
|
if ($this->getArgument('create')) {
|
|
|
|
unset($result[0]['revisionID']);
|
|
|
|
}
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
2012-05-21 01:39:38 +02:00
|
|
|
$result[0] = $this->dispatchWillBuildEvent($result[0]);
|
|
|
|
|
|
|
|
return $result;
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
/**
|
|
|
|
* Convert a list of commits from `getLocalCommitInformation()` into
|
|
|
|
* a format usable by arc to create a new diff. Specifically, we emit:
|
|
|
|
*
|
|
|
|
* - A dictionary of commit message fields.
|
|
|
|
* - A list of errors encountered while parsing the messages.
|
|
|
|
* - A human-readable list of the commits themselves.
|
|
|
|
*
|
|
|
|
* For example, if the user runs "arc diff HEAD^^^" and selects a diff range
|
|
|
|
* which includes several diffs, we attempt to merge them somewhat
|
|
|
|
* intelligently into a single message, because we can only send one
|
|
|
|
* "Summary:", "Reviewers:", etc., field to Differential. We also return
|
|
|
|
* errors (e.g., if the user typed a reviewer name incorrectly) and a
|
|
|
|
* summary of the commits themselves.
|
|
|
|
*
|
|
|
|
* @param dict Local commit information.
|
|
|
|
* @return list Complex output, see summary.
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function parseCommitMessagesIntoFields(array $local) {
|
2012-02-29 02:00:35 +01:00
|
|
|
$conduit = $this->getConduit();
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$local = ipull($local, null, 'commit');
|
2012-02-29 02:00:35 +01:00
|
|
|
|
2012-05-22 23:38:53 +02:00
|
|
|
// If the user provided "--reviewers" or "--ccs", add a faux message to
|
|
|
|
// the list with the implied fields.
|
|
|
|
|
|
|
|
$faux_message = array();
|
|
|
|
if ($this->getArgument('reviewers')) {
|
|
|
|
$faux_message[] = 'Reviewers: '.$this->getArgument('reviewers');
|
|
|
|
}
|
|
|
|
if ($this->getArgument('cc')) {
|
|
|
|
$faux_message[] = 'CC: '.$this->getArgument('cc');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($faux_message) {
|
|
|
|
$faux_message = implode("\n\n", $faux_message);
|
|
|
|
$local = array(
|
|
|
|
'(Flags) ' => array(
|
|
|
|
'message' => $faux_message,
|
|
|
|
'summary' => 'Command-Line Flags',
|
|
|
|
),
|
|
|
|
) + $local;
|
|
|
|
}
|
|
|
|
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
// Build a human-readable list of the commits, so we can show the user which
|
|
|
|
// commits are included in the diff.
|
|
|
|
$included = array();
|
|
|
|
foreach ($local as $hash => $info) {
|
|
|
|
$included[] = substr($hash, 0, 12).' '.$info['summary'];
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
// Parse all of the messages into fields.
|
2012-02-29 02:00:35 +01:00
|
|
|
$messages = array();
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
foreach ($local as $hash => $info) {
|
|
|
|
$text = $info['message'];
|
|
|
|
$obj = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
|
|
|
|
$messages[$hash] = $obj;
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
$notes = array();
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$fields = array();
|
2012-02-29 02:00:35 +01:00
|
|
|
foreach ($messages as $hash => $message) {
|
|
|
|
try {
|
|
|
|
$message->pullDataFromConduit($conduit, $partial = true);
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$fields[$hash] = $message->getFields();
|
2012-02-29 02:00:35 +01:00
|
|
|
} catch (ArcanistDifferentialCommitMessageParserException $ex) {
|
2012-05-04 21:22:47 +02:00
|
|
|
if ($this->getArgument('verbatim')) {
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
// In verbatim mode, just bail when we hit an error. The user can
|
|
|
|
// rerun without --verbatim if they want to fix it manually. Most
|
|
|
|
// users will probably `git commit --amend` instead.
|
2012-05-04 21:22:47 +02:00
|
|
|
throw $ex;
|
|
|
|
}
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$fields[$hash] = $message->getFields();
|
2012-05-04 21:22:47 +02:00
|
|
|
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
$frev = substr($hash, 0, 12);
|
2012-02-29 02:00:35 +01:00
|
|
|
$notes[] = "NOTE: commit {$frev} could not be completely parsed:";
|
|
|
|
foreach ($ex->getParserErrors() as $error) {
|
|
|
|
$notes[] = " - {$error}";
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Improve default fill of Differential messages from commits
Summary:
- When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
- Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
- Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
- @csilvers: this is a precursor to implementing "--reviewers", etc.
Test Plan: See next post, test plan includes giant output.
Reviewers: btrahan, csilvers, Makinde, jungejason, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 20:08:57 +02:00
|
|
|
// Merge commit message fields. We do this somewhat-intelligently so that
|
|
|
|
// multiple "Reviewers" or "CC" fields will merge into the concatenation
|
|
|
|
// of all values.
|
|
|
|
|
|
|
|
// We have special parsing rules for 'title' because we can't merge
|
|
|
|
// multiple titles, and one-line commit messages like "fix stuff" will
|
|
|
|
// parse as titles. Instead, pick the first title we encounter. When we
|
|
|
|
// encounter subsequent titles, treat them as part of the summary. Then
|
|
|
|
// we merge all the summaries together below.
|
|
|
|
|
|
|
|
$result = array();
|
|
|
|
|
|
|
|
// Process fields in oldest-first order, so earlier commits get to set the
|
|
|
|
// title of record and reviewers/ccs are listed in chronological order.
|
|
|
|
$fields = array_reverse($fields);
|
|
|
|
|
|
|
|
foreach ($fields as $hash => $dict) {
|
|
|
|
$title = idx($dict, 'title');
|
|
|
|
if (!strlen($title)) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!isset($result['title'])) {
|
|
|
|
// We don't have a title yet, so use this one.
|
|
|
|
$result['title'] = $title;
|
|
|
|
} else {
|
|
|
|
// We already have a title, so merge this new title into the summary.
|
|
|
|
$summary = idx($dict, 'summary');
|
|
|
|
if ($summary) {
|
|
|
|
$summary = $title."\n\n".$summary;
|
|
|
|
} else {
|
|
|
|
$summary = $title;
|
|
|
|
}
|
|
|
|
$fields[$hash]['summary'] = $summary;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Now, merge all the other fields in a general sort of way.
|
|
|
|
|
|
|
|
foreach ($fields as $hash => $dict) {
|
|
|
|
foreach ($dict as $key => $value) {
|
|
|
|
if ($key == 'title') {
|
|
|
|
// This has been handled above, and either assigned directly or
|
|
|
|
// merged into the summary.
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (is_array($value)) {
|
|
|
|
// For array values, merge the arrays, appending the new values.
|
|
|
|
// Examples are "Reviewers" and "Cc", where this produces a list of
|
|
|
|
// all users specified as reviewers.
|
|
|
|
$cur = idx($result, $key, array());
|
|
|
|
$new = array_merge($cur, $value);
|
|
|
|
$result[$key] = $new;
|
|
|
|
continue;
|
|
|
|
} else {
|
|
|
|
if (!strlen(trim($value))) {
|
|
|
|
// Ignore empty fields.
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
// For string values, append the new field to the old field with
|
|
|
|
// a blank line separating them. Examples are "Test Plan" and
|
|
|
|
// "Summary".
|
|
|
|
$cur = idx($result, $key, '');
|
|
|
|
if (strlen($cur)) {
|
|
|
|
$new = $cur."\n\n".$value;
|
|
|
|
} else {
|
|
|
|
$new = $value;
|
|
|
|
}
|
|
|
|
$result[$key] = $new;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return array($result, $notes, $included);
|
2012-02-29 02:00:35 +01:00
|
|
|
}
|
|
|
|
|
2012-02-23 18:18:49 +01:00
|
|
|
private function getDefaultUpdateMessage() {
|
2012-03-05 19:08:50 +01:00
|
|
|
if ($this->isRawDiffSource()) {
|
2012-02-23 18:18:49 +01:00
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
if ($repository_api instanceof ArcanistGitAPI) {
|
|
|
|
return $this->getGitUpdateMessage();
|
|
|
|
}
|
|
|
|
|
2012-02-29 01:59:40 +01:00
|
|
|
if ($repository_api instanceof ArcanistMercurialAPI) {
|
|
|
|
return $this->getMercurialUpdateMessage();
|
|
|
|
}
|
|
|
|
|
2012-02-23 18:18:49 +01:00
|
|
|
return null;
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
/**
|
2012-02-29 01:59:40 +01:00
|
|
|
* Retrieve the git messages between HEAD and the last update.
|
2012-01-05 20:26:17 +01:00
|
|
|
*
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function getGitUpdateMessage() {
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
2012-07-18 01:16:18 +02:00
|
|
|
$parser = $this->newDiffParser();
|
2012-01-05 20:26:17 +01:00
|
|
|
$commit_messages = $repository_api->getGitCommitLog();
|
|
|
|
$commit_messages = $parser->parseDiff($commit_messages);
|
|
|
|
|
2012-02-23 18:18:49 +01:00
|
|
|
if (count($commit_messages) == 1) {
|
|
|
|
// If there's only one message, assume this is an amend-based workflow and
|
|
|
|
// that using it to prefill doesn't make sense.
|
2012-01-05 20:26:17 +01:00
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-02-23 18:18:49 +01:00
|
|
|
// We have more than one message, so figure out which ones are new. We
|
|
|
|
// do this by pulling the current diff and comparing commit hashes in the
|
|
|
|
// working copy with attached commit hashes. It's not super important that
|
|
|
|
// we always get this 100% right, we're just trying to do something
|
|
|
|
// reasonable.
|
|
|
|
|
2012-02-29 01:59:40 +01:00
|
|
|
$local = $this->loadActiveLocalCommitInfo();
|
2012-02-23 18:18:49 +01:00
|
|
|
$hashes = ipull($local, null, 'commit');
|
|
|
|
|
|
|
|
$usable = array();
|
|
|
|
foreach ($commit_messages as $message) {
|
|
|
|
$text = $message->getMetadata('message');
|
|
|
|
|
|
|
|
$parsed = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
|
|
|
|
if ($parsed->getRevisionID()) {
|
|
|
|
// If this is an amended commit message with a revision ID, it's
|
|
|
|
// certainly not new. Stop marking commits as usable and break out.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (isset($hashes[$message->getCommitHash()])) {
|
|
|
|
// If this commit is currently part of the diff, stop using commit
|
|
|
|
// messages, since anything older than this isn't new.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Otherwise, this looks new, so it's a usable commit message.
|
2012-04-07 19:36:16 +02:00
|
|
|
$usable[] = $text;
|
2012-02-23 18:18:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if (!$usable) {
|
|
|
|
// No new commit messages, so we don't have anywhere to start from.
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-02-29 01:59:40 +01:00
|
|
|
return $this->formatUsableLogs($usable);
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Retrieve the hg messages between tip and the last update.
|
|
|
|
*
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function getMercurialUpdateMessage() {
|
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
|
|
|
$messages = $repository_api->getCommitMessageLog();
|
|
|
|
|
2013-06-17 20:40:58 +02:00
|
|
|
if (count($messages) == 1) {
|
|
|
|
// If there's only one message, assume this is an amend-based workflow and
|
|
|
|
// that using it to prefill doesn't make sense.
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2012-02-29 01:59:40 +01:00
|
|
|
$local = $this->loadActiveLocalCommitInfo();
|
2012-05-10 20:43:21 +02:00
|
|
|
$hashes = ipull($local, null, 'commit');
|
2012-02-29 01:59:40 +01:00
|
|
|
|
|
|
|
$usable = array();
|
|
|
|
foreach ($messages as $rev => $message) {
|
|
|
|
if (isset($hashes[$rev])) {
|
|
|
|
// If this commit is currently part of the active diff on the revision,
|
|
|
|
// stop using commit messages, since anything older than this isn't new.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Otherwise, this looks new, so it's a usable commit message.
|
|
|
|
$usable[] = $message;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$usable) {
|
|
|
|
// No new commit messages, so we don't have anywhere to start from.
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $this->formatUsableLogs($usable);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Format log messages to prefill a diff update.
|
|
|
|
*
|
|
|
|
* @task message
|
|
|
|
*/
|
|
|
|
private function formatUsableLogs(array $usable) {
|
2012-02-23 18:18:49 +01:00
|
|
|
// Flip messages so they'll read chronologically (oldest-first) in the
|
|
|
|
// template, e.g.:
|
|
|
|
//
|
|
|
|
// - Added foobar.
|
|
|
|
// - Fixed foobar bug.
|
|
|
|
// - Documented foobar.
|
|
|
|
|
|
|
|
$usable = array_reverse($usable);
|
|
|
|
$default = array();
|
|
|
|
foreach ($usable as $message) {
|
|
|
|
// Pick the first line out of each message.
|
2012-02-29 01:59:40 +01:00
|
|
|
$text = trim($message);
|
2012-02-23 18:18:49 +01:00
|
|
|
$text = head(explode("\n", $text));
|
|
|
|
$default[] = ' - '.$text."\n";
|
|
|
|
}
|
|
|
|
|
|
|
|
return implode('', $default);
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
2012-02-29 01:59:40 +01:00
|
|
|
private function loadActiveLocalCommitInfo() {
|
|
|
|
$current_diff = $this->getConduit()->callMethodSynchronous(
|
2015-02-03 23:18:10 +01:00
|
|
|
'differential.querydiffs',
|
2012-02-29 01:59:40 +01:00
|
|
|
array(
|
2015-02-03 23:18:10 +01:00
|
|
|
'revisionIDs' => array($this->revisionID),
|
2012-02-29 01:59:40 +01:00
|
|
|
));
|
2015-02-03 23:18:10 +01:00
|
|
|
$current_diff = head($current_diff);
|
2012-02-29 01:59:40 +01:00
|
|
|
|
|
|
|
$properties = idx($current_diff, 'properties', array());
|
|
|
|
return idx($properties, 'local:commits', array());
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
|
|
|
|
/* -( Diff Specification )------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task diffspec
|
|
|
|
*/
|
|
|
|
private function getLintStatus($lint_result) {
|
|
|
|
$map = array(
|
|
|
|
ArcanistLintWorkflow::RESULT_OKAY => 'okay',
|
|
|
|
ArcanistLintWorkflow::RESULT_ERRORS => 'fail',
|
|
|
|
ArcanistLintWorkflow::RESULT_WARNINGS => 'warn',
|
|
|
|
ArcanistLintWorkflow::RESULT_SKIP => 'skip',
|
2012-07-03 00:53:22 +02:00
|
|
|
ArcanistLintWorkflow::RESULT_POSTPONED => 'postponed',
|
2012-01-05 20:26:17 +01:00
|
|
|
);
|
|
|
|
return idx($map, $lint_result, 'none');
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task diffspec
|
|
|
|
*/
|
|
|
|
private function getUnitStatus($unit_result) {
|
|
|
|
$map = array(
|
|
|
|
ArcanistUnitWorkflow::RESULT_OKAY => 'okay',
|
|
|
|
ArcanistUnitWorkflow::RESULT_FAIL => 'fail',
|
|
|
|
ArcanistUnitWorkflow::RESULT_UNSOUND => 'warn',
|
|
|
|
ArcanistUnitWorkflow::RESULT_SKIP => 'skip',
|
|
|
|
ArcanistUnitWorkflow::RESULT_POSTPONED => 'postponed',
|
|
|
|
);
|
|
|
|
return idx($map, $unit_result, 'none');
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task diffspec
|
|
|
|
*/
|
|
|
|
private function buildDiffSpecification() {
|
|
|
|
|
2012-01-05 22:36:24 +01:00
|
|
|
$base_revision = null;
|
|
|
|
$base_path = null;
|
|
|
|
$vcs = null;
|
2012-01-05 20:26:17 +01:00
|
|
|
$repo_uuid = null;
|
|
|
|
$parent = null;
|
2012-01-05 22:36:24 +01:00
|
|
|
$source_path = null;
|
|
|
|
$branch = null;
|
2012-07-01 01:00:48 +02:00
|
|
|
$bookmark = null;
|
2012-01-05 22:36:24 +01:00
|
|
|
|
2012-03-05 19:08:50 +01:00
|
|
|
if (!$this->isRawDiffSource()) {
|
2012-01-05 22:36:24 +01:00
|
|
|
$repository_api = $this->getRepositoryAPI();
|
|
|
|
|
|
|
|
$base_revision = $repository_api->getSourceControlBaseRevision();
|
|
|
|
$base_path = $repository_api->getSourceControlPath();
|
|
|
|
$vcs = $repository_api->getSourceControlSystemName();
|
|
|
|
$source_path = $repository_api->getPath();
|
|
|
|
$branch = $repository_api->getBranchName();
|
Allow `arc` to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:
- Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
- Enhance `arc which` to explain the process to the user.
- The `project_id` key is no longer required in `.arcconfig`.
Minor/cleanup changes:
- Rename `project_id` to `project.name` (consistency, clarity).
- Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
- These both need documentation updates.
- Add `repository.callsign` to explicitly bind to a repository.
- Updated `.arcconfig` for the new values.
- Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
- Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.
Test Plan:
- Ran `arc which`.
- Ran `arc diff`.
- This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4343
Differential Revision: https://secure.phabricator.com/D8073
2014-01-27 00:31:30 +01:00
|
|
|
$repo_uuid = $repository_api->getRepositoryUUID();
|
2012-01-05 22:36:24 +01:00
|
|
|
|
|
|
|
if ($repository_api instanceof ArcanistGitAPI) {
|
|
|
|
$info = $this->getGitParentLogInfo();
|
|
|
|
if ($info['parent']) {
|
|
|
|
$parent = $info['parent'];
|
|
|
|
}
|
|
|
|
if ($info['base_revision']) {
|
|
|
|
$base_revision = $info['base_revision'];
|
|
|
|
}
|
|
|
|
if ($info['base_path']) {
|
|
|
|
$base_path = $info['base_path'];
|
|
|
|
}
|
|
|
|
if ($info['uuid']) {
|
|
|
|
$repo_uuid = $info['uuid'];
|
|
|
|
}
|
|
|
|
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
2012-07-01 01:00:48 +02:00
|
|
|
|
|
|
|
$bookmark = $repository_api->getActiveBookmark();
|
|
|
|
$svn_info = $repository_api->getSubversionInfo();
|
|
|
|
$repo_uuid = idx($svn_info, 'uuid');
|
|
|
|
$base_path = idx($svn_info, 'base_path', $base_path);
|
|
|
|
$base_revision = idx($svn_info, 'base_revision', $base_revision);
|
|
|
|
|
|
|
|
// TODO: provide parent info
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
2012-01-05 22:36:24 +01:00
|
|
|
}
|
|
|
|
|
Allow `arc` to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:
- Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
- Enhance `arc which` to explain the process to the user.
- The `project_id` key is no longer required in `.arcconfig`.
Minor/cleanup changes:
- Rename `project_id` to `project.name` (consistency, clarity).
- Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
- These both need documentation updates.
- Add `repository.callsign` to explicitly bind to a repository.
- Updated `.arcconfig` for the new values.
- Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
- Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.
Test Plan:
- Ran `arc which`.
- Ran `arc diff`.
- This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4343
Differential Revision: https://secure.phabricator.com/D8073
2014-01-27 00:31:30 +01:00
|
|
|
$data = array(
|
2012-01-05 20:26:17 +01:00
|
|
|
'sourceMachine' => php_uname('n'),
|
2012-01-05 22:36:24 +01:00
|
|
|
'sourcePath' => $source_path,
|
|
|
|
'branch' => $branch,
|
2012-07-01 01:00:48 +02:00
|
|
|
'bookmark' => $bookmark,
|
2012-01-05 20:26:17 +01:00
|
|
|
'sourceControlSystem' => $vcs,
|
|
|
|
'sourceControlPath' => $base_path,
|
|
|
|
'sourceControlBaseRevision' => $base_revision,
|
|
|
|
'creationMethod' => 'arc',
|
|
|
|
);
|
Allow `arc` to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:
- Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
- Enhance `arc which` to explain the process to the user.
- The `project_id` key is no longer required in `.arcconfig`.
Minor/cleanup changes:
- Rename `project_id` to `project.name` (consistency, clarity).
- Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
- These both need documentation updates.
- Add `repository.callsign` to explicitly bind to a repository.
- Updated `.arcconfig` for the new values.
- Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
- Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.
Test Plan:
- Ran `arc which`.
- Ran `arc diff`.
- This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4343
Differential Revision: https://secure.phabricator.com/D8073
2014-01-27 00:31:30 +01:00
|
|
|
|
2014-05-16 04:03:29 +02:00
|
|
|
if (!$this->isRawDiffSource()) {
|
|
|
|
$repository_phid = $this->getRepositoryPHID();
|
|
|
|
if ($repository_phid) {
|
|
|
|
$data['repositoryPHID'] = $repository_phid;
|
|
|
|
}
|
Allow `arc` to identify repositories without "project_id"
Summary:
Ref T4343. Continues the process of reducing the prominence of Arcanist Projects. Primarily:
- Query Phabricator to identify the working copy based on explicit configuration, or guess based on heuristics.
- Enhance `arc which` to explain the process to the user.
- The `project_id` key is no longer required in `.arcconfig`.
Minor/cleanup changes:
- Rename `project_id` to `project.name` (consistency, clarity).
- Rename `conduit_uri` to `phabricator.uri` (consistency, clairty).
- These both need documentation updates.
- Add `repository.callsign` to explicitly bind to a repository.
- Updated `.arcconfig` for the new values.
- Fix a unit test which broke a while ago when we fixed a rare definition of "unstaged".
- Make `getRepositoryUUID()` generic so we can get rid of one `instanceof`.
Test Plan:
- Ran `arc which`.
- Ran `arc diff`.
- This doesn't really change anything, so the only real risk is version compatibility breaks. This //does// introduce such a break, but the window is very narrow: if you upgrade `arc` after this commit, and try to diff against a Phabricator which was updated after yesterday (D8068) but before D8072 lands, the lookup will work so we'll add `repositoryPHID` to the `differential.creatediff` call, but it won't exist in Phabricator yet. This window is so narrow that I'm not going to try to fix it, as I'd guess there is a significant chance that no users will be affected. I don't see a clever way to fix it that doesn't involve a lot of work, either.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4343
Differential Revision: https://secure.phabricator.com/D8073
2014-01-27 00:31:30 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return $data;
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/* -( Diff Properties )---------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Update lint information for the diff.
|
|
|
|
*
|
|
|
|
* @return void
|
|
|
|
*
|
|
|
|
* @task diffprop
|
|
|
|
*/
|
|
|
|
private function updateLintDiffProperty() {
|
2012-11-02 06:41:00 +01:00
|
|
|
if (strlen($this->excuses['lint'])) {
|
|
|
|
$this->updateDiffProperty('arc:lint-excuse',
|
|
|
|
json_encode($this->excuses['lint']));
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
|
2012-07-03 00:53:22 +02:00
|
|
|
if ($this->unresolvedLint) {
|
2012-08-06 22:40:20 +02:00
|
|
|
$this->updateDiffProperty('arc:lint', json_encode($this->unresolvedLint));
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
2012-07-03 00:53:22 +02:00
|
|
|
$postponed = $this->postponedLinters;
|
|
|
|
if ($postponed) {
|
|
|
|
$this->updateDiffProperty('arc:lint-postponed', json_encode($postponed));
|
2012-02-23 00:57:55 +01:00
|
|
|
}
|
2012-07-03 00:53:22 +02:00
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Update unit test information for the diff.
|
|
|
|
*
|
|
|
|
* @return void
|
|
|
|
*
|
|
|
|
* @task diffprop
|
|
|
|
*/
|
|
|
|
private function updateUnitDiffProperty() {
|
2012-10-22 23:46:31 +02:00
|
|
|
if (strlen($this->excuses['unit'])) {
|
2012-02-23 00:57:55 +01:00
|
|
|
$this->updateDiffProperty('arc:unit-excuse',
|
2012-10-22 23:46:31 +02:00
|
|
|
json_encode($this->excuses['unit']));
|
2012-02-23 00:57:55 +01:00
|
|
|
}
|
2012-11-02 06:41:00 +01:00
|
|
|
|
|
|
|
if ($this->testResults) {
|
|
|
|
$this->updateDiffProperty('arc:unit', json_encode($this->testResults));
|
|
|
|
}
|
2012-01-05 20:26:17 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Update local commit information for the diff.
|
|
|
|
*
|
|
|
|
* @task diffprop
|
|
|
|
*/
|
|
|
|
private function updateLocalDiffProperty() {
|
2012-01-05 22:36:24 +01:00
|
|
|
if ($this->isRawDiffSource()) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
2012-01-05 20:26:17 +01:00
|
|
|
$local_info = $this->getRepositoryAPI()->getLocalCommitInformation();
|
|
|
|
if (!$local_info) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$this->updateDiffProperty('local:commits', json_encode($local_info));
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Update an arbitrary diff property.
|
|
|
|
*
|
|
|
|
* @param string Diff property name.
|
|
|
|
* @param string Diff property value.
|
|
|
|
* @return void
|
|
|
|
*
|
|
|
|
* @task diffprop
|
|
|
|
*/
|
|
|
|
private function updateDiffProperty($name, $data) {
|
2012-10-13 00:01:19 +02:00
|
|
|
$this->diffPropertyFutures[] = $this->getConduit()->callMethod(
|
2012-01-05 20:26:17 +01:00
|
|
|
'differential.setdiffproperty',
|
|
|
|
array(
|
|
|
|
'diff_id' => $this->getDiffID(),
|
|
|
|
'name' => $name,
|
|
|
|
'data' => $data,
|
|
|
|
));
|
|
|
|
}
|
|
|
|
|
2012-10-13 00:01:19 +02:00
|
|
|
/**
|
|
|
|
* Wait for finishing all diff property updates.
|
|
|
|
*
|
|
|
|
* @return void
|
|
|
|
*
|
|
|
|
* @task diffprop
|
|
|
|
*/
|
|
|
|
private function resolveDiffPropertyUpdates() {
|
2014-12-30 13:14:32 +01:00
|
|
|
id(new FutureIterator($this->diffPropertyFutures))
|
|
|
|
->resolveAll();
|
2012-10-13 00:01:19 +02:00
|
|
|
$this->diffPropertyFutures = array();
|
|
|
|
}
|
|
|
|
|
2012-08-30 21:28:35 +02:00
|
|
|
private function dispatchWillCreateRevisionEvent(array $fields) {
|
2012-09-04 21:08:04 +02:00
|
|
|
$event = $this->dispatchEvent(
|
2012-08-30 21:28:35 +02:00
|
|
|
ArcanistEventType::TYPE_REVISION_WILLCREATEREVISION,
|
|
|
|
array(
|
|
|
|
'specification' => $fields,
|
|
|
|
));
|
|
|
|
|
|
|
|
return $event->getValue('specification');
|
|
|
|
}
|
|
|
|
|
2012-05-21 01:39:38 +02:00
|
|
|
private function dispatchWillBuildEvent(array $fields) {
|
2012-09-04 21:08:04 +02:00
|
|
|
$event = $this->dispatchEvent(
|
2012-05-21 01:39:38 +02:00
|
|
|
ArcanistEventType::TYPE_DIFF_WILLBUILDMESSAGE,
|
|
|
|
array(
|
2012-08-30 21:28:35 +02:00
|
|
|
'fields' => $fields,
|
2012-05-21 01:39:38 +02:00
|
|
|
));
|
|
|
|
|
|
|
|
return $event->getValue('fields');
|
|
|
|
}
|
|
|
|
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
private function checkRevisionOwnership(array $revision) {
|
|
|
|
if ($revision['authorPHID'] == $this->getUserPHID()) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$id = $revision['id'];
|
|
|
|
$title = $revision['title'];
|
|
|
|
|
|
|
|
throw new ArcanistUsageException(
|
2015-05-13 10:05:15 +02:00
|
|
|
pht(
|
|
|
|
"You don't own revision %s '%s'. You can only update revisions ".
|
|
|
|
"you own. You can 'Commandeer' this revision from the web ".
|
|
|
|
"interface if you want to become the owner.",
|
|
|
|
"D{$id}",
|
|
|
|
$title));
|
In `arc diff`, always check that you own a revision before trying to do anything with it
Summary:
Some users assume they can update anything, not just revisions they own (see https://github.com/facebook/arcanist/issues/54).
Currently, if you `arc patch` or `arc amend` and get a commit message, then `arc diff` for a revision you don't own, we:
- Fail early with a very confusing message ("You can not review a revision you own!") if you are on the "Reviewers" line, until D3820.
- Or fail very very late with a good error message, but after lint, unit and update messages.
Instead, check that you own the revision as early as we can.
Test Plan: Tried to update revisions I didn't own, got good error messages early on (with D3820).
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3821
2012-10-26 01:27:57 +02:00
|
|
|
}
|
|
|
|
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
|
|
|
|
/* -( File Uploads )------------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
private function uploadFilesForChanges(array $changes) {
|
|
|
|
assert_instances_of($changes, 'ArcanistDiffChange');
|
|
|
|
|
|
|
|
// Collect all the files we need to upload.
|
|
|
|
|
|
|
|
$need_upload = array();
|
|
|
|
foreach ($changes as $key => $change) {
|
|
|
|
if ($change->getFileType() != ArcanistDiffChangeType::FILE_BINARY) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->getArgument('skip-binaries')) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$name = basename($change->getCurrentPath());
|
|
|
|
|
|
|
|
$need_upload[] = array(
|
|
|
|
'type' => 'old',
|
|
|
|
'name' => $name,
|
|
|
|
'data' => $change->getOriginalFileData(),
|
|
|
|
'change' => $change,
|
|
|
|
);
|
|
|
|
|
|
|
|
$need_upload[] = array(
|
|
|
|
'type' => 'new',
|
|
|
|
'name' => $name,
|
|
|
|
'data' => $change->getCurrentFileData(),
|
|
|
|
'change' => $change,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$need_upload) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
// Determine mime types and file sizes. Update changes from "binary" to
|
|
|
|
// "image" if the file is an image. Set image metadata.
|
|
|
|
|
|
|
|
$type_image = ArcanistDiffChangeType::FILE_IMAGE;
|
|
|
|
foreach ($need_upload as $key => $spec) {
|
|
|
|
$change = $need_upload[$key]['change'];
|
|
|
|
|
|
|
|
$type = $spec['type'];
|
|
|
|
$size = strlen($spec['data']);
|
|
|
|
|
|
|
|
$change->setMetadata("{$type}:file:size", $size);
|
|
|
|
if ($spec['data'] === null) {
|
|
|
|
// This covers the case where a file was added or removed; we don't
|
When converting a file to a binary, populate the binary's data
Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.
A simple reproduction case is to build a test file (I used one with bytes from 1..255):
$ # Don't include \0, since Git treats that specially.
$ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt
Then add it:
$ git add example.txt
$ git commit -a -m derp
$ arc diff --only HEAD^
You'll be prompted to convert the file to binary:
Do you want to mark this file as binary and continue? [Y/n] y
Before this patch, that would be followed by:
Uploading 0 files...
...which is incorrect; we need to upload the new data. After this patch, this shows:
Uploading 1 files...
...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.
Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6815
2013-08-27 18:34:30 +02:00
|
|
|
// need to upload the other half of it (e.g., the old file data for
|
|
|
|
// a file which was just added). This is distinct from an empty
|
|
|
|
// file, which we do upload.
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
unset($need_upload[$key]);
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$mime = $this->getFileMimeType($spec['data']);
|
|
|
|
if (preg_match('@^image/@', $mime)) {
|
|
|
|
$change->setFileType($type_image);
|
|
|
|
}
|
|
|
|
|
|
|
|
$change->setMetadata("{$type}:file:mime-type", $mime);
|
|
|
|
}
|
|
|
|
|
2014-05-23 22:53:05 +02:00
|
|
|
echo pht('Uploading %d files...', count($need_upload))."\n";
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
|
2015-01-02 00:10:10 +01:00
|
|
|
$hash_futures = array();
|
|
|
|
foreach ($need_upload as $key => $spec) {
|
|
|
|
$hash_futures[$key] = $this->getConduit()->callMethod(
|
|
|
|
'file.uploadhash',
|
|
|
|
array(
|
|
|
|
'name' => $spec['name'],
|
|
|
|
'hash' => sha1($spec['data']),
|
|
|
|
));
|
|
|
|
}
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
|
2015-01-02 00:10:10 +01:00
|
|
|
$futures = id(new FutureIterator($hash_futures))
|
|
|
|
->limit(8);
|
|
|
|
foreach ($futures as $key => $future) {
|
|
|
|
$type = $need_upload[$key]['type'];
|
|
|
|
$change = $need_upload[$key]['change'];
|
|
|
|
$name = $need_upload[$key]['name'];
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
|
2015-01-02 00:10:10 +01:00
|
|
|
$phid = null;
|
|
|
|
try {
|
|
|
|
$phid = $future->resolve();
|
|
|
|
} catch (Exception $e) {
|
|
|
|
// Just try uploading normally if the hash upload failed.
|
|
|
|
continue;
|
|
|
|
}
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
|
2015-01-02 00:10:10 +01:00
|
|
|
if ($phid) {
|
|
|
|
$change->setMetadata("{$type}:binary-phid", $phid);
|
|
|
|
unset($need_upload[$key]);
|
|
|
|
echo pht("Uploaded '%s' (%s).", $name, $type)."\n";
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$upload_futures = array();
|
|
|
|
foreach ($need_upload as $key => $spec) {
|
|
|
|
$upload_futures[$key] = $this->getConduit()->callMethod(
|
|
|
|
'file.upload',
|
|
|
|
array(
|
|
|
|
'name' => $spec['name'],
|
|
|
|
'data_base64' => base64_encode($spec['data']),
|
|
|
|
));
|
|
|
|
}
|
|
|
|
|
2014-12-30 13:14:32 +01:00
|
|
|
$futures = id(new FutureIterator($upload_futures))
|
|
|
|
->limit(4);
|
|
|
|
foreach ($futures as $key => $future) {
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
$type = $need_upload[$key]['type'];
|
|
|
|
$change = $need_upload[$key]['change'];
|
|
|
|
$name = $need_upload[$key]['name'];
|
|
|
|
|
|
|
|
try {
|
|
|
|
$phid = $future->resolve();
|
|
|
|
$change->setMetadata("{$type}:binary-phid", $phid);
|
|
|
|
echo pht("Uploaded '%s' (%s).", $name, $type)."\n";
|
|
|
|
} catch (Exception $e) {
|
2015-05-13 10:05:15 +02:00
|
|
|
echo pht("Failed to upload %s binary '%s'.", $type, $name)."\n\n";
|
2013-06-12 14:50:08 +02:00
|
|
|
echo $e->getMessage()."\n";
|
2015-05-13 10:05:15 +02:00
|
|
|
if (!phutil_console_confirm(pht('Continue?'), $default_no = false)) {
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
throw new ArcanistUsageException(
|
2015-05-13 10:05:15 +02:00
|
|
|
pht(
|
|
|
|
'Aborted due to file upload failure. You can use %s '.
|
|
|
|
'to skip binary uploads.',
|
|
|
|
'--skip-binaries'));
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-05-23 22:53:05 +02:00
|
|
|
echo pht('Upload complete.')."\n";
|
Simplify (?) arc upload code and make it more future-oriented
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.
Test Plan:
Created a binary change in a commit:
$ head -c65535 /dev/urandom > a_random_file
$ git add .
$ git commit -m .
[master 32e4c0a] .
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a_random_file
Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 111,500 us
<<< [15] <conduit> 112,169 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 85,386 us
<<< [17] <conduit> 85,798 us
>>> [19] <conduit> file.upload() <bytes = 97009>
>>> [20] <http> http://local.aphront.com/api/file.upload
<<< [20] <http> 144,098 us
<<< [19] <conduit> 144,550 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/202/
Included changes:
A (bin) a_random_file
Created the same diff again, verified that we skip the data transfer this time:
$ arc diff HEAD^ --only --trace
...
Uploading 1 files...
>>> [15] <conduit> conduit.query() <bytes = 157>
>>> [16] <http> http://local.aphront.com/api/conduit.query
<<< [16] <http> 112,717 us
<<< [15] <conduit> 113,374 us
>>> [17] <conduit> file.uploadhash() <bytes = 254>
>>> [18] <http> http://local.aphront.com/api/file.uploadhash
<<< [18] <http> 95,545 us
<<< [17] <conduit> 95,921 us
Uploaded 'a_random_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/203/
Included changes:
A (bin) a_random_file
Created a new commit which adds a new file and updates the exiting file:
$ head -c65535 /dev/urandom > another_file
$ head -c65535 /dev/urandom >> a_random_file
$ git add .
$ git commit -m .
[master 28f4fbd] .
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 another_file
Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 114,825 us
<<< [19] <conduit> 115,517 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [24] <http> 104,310 us
<<< [26] <http> 105,211 us
<<< [25] <conduit> 105,587 us
<<< [22] <http> 112,631 us
<<< [25] <conduit> 111,437 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 111,678 us
>>> [27] <conduit> file.upload() <bytes = 193912>
>>> [28] <http> http://local.aphront.com/api/file.upload
>>> [29] <conduit> file.upload() <bytes = 97379>
>>> [30] <http> http://local.aphront.com/api/file.upload
<<< [28] <http> 153,126 us
<<< [29] <conduit> 161,180 us
Uploaded 'a_random_file' (new).
<<< [30] <http> 678,739 us
<<< [29] <conduit> 679,121 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/204/
Included changes:
M (bin) a_random_file
A (bin) another_file
Ran the same command again, verified three uploads by hash:
$ arc diff HEAD^ --only --trace
...
Uploading 3 files...
>>> [19] <conduit> conduit.query() <bytes = 157>
>>> [20] <http> http://local.aphront.com/api/conduit.query
<<< [20] <http> 117,058 us
<<< [19] <conduit> 117,792 us
>>> [21] <conduit> file.uploadhash() <bytes = 254>
>>> [22] <http> http://local.aphront.com/api/file.uploadhash
>>> [23] <conduit> file.uploadhash() <bytes = 254>
>>> [24] <http> http://local.aphront.com/api/file.uploadhash
>>> [25] <conduit> file.uploadhash() <bytes = 253>
>>> [26] <http> http://local.aphront.com/api/file.uploadhash
<<< [22] <http> 103,373 us
<<< [24] <http> 105,418 us
<<< [26] <http> 105,251 us
<<< [25] <conduit> 105,604 us
Uploaded 'a_random_file' (old).
<<< [25] <conduit> 105,844 us
Uploaded 'a_random_file' (new).
<<< [25] <conduit> 106,053 us
Uploaded 'another_file' (new).
Upload complete.
...
Created a new Differential diff:
Diff URI: http://local.aphront.com:8080/differential/diff/205/
Included changes:
M (bin) a_random_file
A (bin) another_file
$
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2456
Differential Revision: https://secure.phabricator.com/D4993
2013-02-20 21:24:32 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
private function getFileMimeType($data) {
|
|
|
|
$tmp = new TempFile();
|
|
|
|
Filesystem::writeFile($tmp, $data);
|
|
|
|
return Filesystem::getMimeType($tmp);
|
|
|
|
}
|
|
|
|
|
2014-08-04 21:03:03 +02:00
|
|
|
private function shouldOpenCreatedObjectsInBrowser() {
|
|
|
|
return $this->getArgument('browse');
|
|
|
|
}
|
|
|
|
|
2011-01-10 00:22:25 +01:00
|
|
|
}
|