From d952502f12b41d02ab37ba48c052135c34327f27 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 5 Feb 2013 20:11:39 -0800 Subject: [PATCH] Make "arc patch" read `authorName` and `authorEmail` Summary: If provided, have `arc patch` use `authorName` / `authorEmail`. This simplifies handling and makes patches more portable between version control systems (previously, information was generated in the diff's VCS, regardless of which VCS it was being applied to). Test Plan: Created a diff with author `derp `, ran `arc patch --diff x`, got a local commit with that author. Reviewers: btrahan, edward, vrana Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D4827 --- src/parser/ArcanistBundle.php | 54 +++++++++++++++++++++----- src/workflow/ArcanistBaseWorkflow.php | 3 +- src/workflow/ArcanistPatchWorkflow.php | 32 +++++++-------- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/parser/ArcanistBundle.php b/src/parser/ArcanistBundle.php index c0000c9f..6e46b8e8 100644 --- a/src/parser/ArcanistBundle.php +++ b/src/parser/ArcanistBundle.php @@ -16,14 +16,48 @@ final class ArcanistBundle { private $revisionID; private $encoding; private $loadFileDataCallback; - private $author; + private $authorName; + private $authorEmail; - public function setAuthor($author) { - $this->author = $author; + public function setAuthorEmail($author_email) { + $this->authorEmail = $author_email; return $this; } - public function getAuthor() { - return $this->author; + + public function getAuthorEmail() { + return $this->authorEmail; + } + + public function setAuthorName($author_name) { + $this->authorName = $author_name; + return $this; + } + + public function getAuthorName() { + return $this->authorName; + } + + public function getFullAuthor() { + $author_name = $this->getAuthorName(); + if ($author_name === null) { + return null; + } + + $author_email = $this->getAuthorEmail(); + if ($author_email === null) { + return null; + } + + $full_author = sprintf('%s <%s>', $author_name, $author_email); + + // Because git is very picky about the author being in a valid format, + // verify that we can parse it. + $address = new PhutilEmailAddress($full_author); + if (!$address->getDisplayName() || !$address->getAddress()) { + return null; + } + + return $full_author; } public function setConduit(ConduitClient $conduit) { @@ -116,9 +150,10 @@ final class ArcanistBundle { $base_revision = idx($meta_info, 'baseRevision'); $revision_id = idx($meta_info, 'revisionID'); $encoding = idx($meta_info, 'encoding'); - $author = idx($meta_info, 'author'); - // this arc bundle was probably made before we started storing meta info + $author_name = idx($meta_info, 'authorName'); + $author_email = idx($meta_info, 'authorEmail'); } else { + // this arc bundle was probably made before we started storing meta info $version = 0; $project_name = null; $base_revision = null; @@ -197,12 +232,13 @@ final class ArcanistBundle { } $meta_info = array( - 'version' => 4, + 'version' => 5, 'projectName' => $this->getProjectID(), 'baseRevision' => $this->getBaseRevision(), 'revisionID' => $this->getRevisionID(), 'encoding' => $this->getEncoding(), - 'author' => $this->getAuthor(), + 'authorName' => $this->getAuthorName(), + 'authorEmail' => $this->getAuthorEmail(), ); $dir = Filesystem::createTemporaryDirectory(); diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index 795555d2..72d5659e 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -970,7 +970,8 @@ abstract class ArcanistBaseWorkflow extends Phobject { $bundle->setProjectID(idx($diff, 'projectName')); $bundle->setBaseRevision(idx($diff, 'sourceControlBaseRevision')); $bundle->setRevisionID(idx($diff, 'revisionID')); - $bundle->setAuthor(idx($diff, 'author')); + $bundle->setAuthorName(idx($diff, 'authorName')); + $bundle->setAuthorEmail(idx($diff, 'authorEmail')); return $bundle; } diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php index 2d9c44fd..e9fa45a9 100644 --- a/src/workflow/ArcanistPatchWorkflow.php +++ b/src/workflow/ArcanistPatchWorkflow.php @@ -706,23 +706,15 @@ EOTEXT } if ($this->shouldCommit()) { - // git is really, really finicky about the author information so we - // make sure it is in the - // "George Washington format or don't use it. - $author_cmd = ''; - $author = $bundle->getAuthor(); - if ($author) { - $address = new PhutilEmailAddress($author); - if ($address->getDisplayName() && $address->getAddress()) { - $author = sprintf('%s <%s>', - $address->getDisplayName(), - $address->getAddress()); - $author_cmd = csprintf('--author=%s ', $author); - } + if ($bundle->getFullAuthor()) { + $author_cmd = csprintf('--author=%s', $bundle->getFullAuthor()); + } else { + $author_cmd = ''; } + $commit_message = $this->getCommitMessage($bundle); $future = $repository_api->execFutureLocal( - 'commit -a %C-F -', + 'commit -a %C -F -', $author_cmd); $future->write($commit_message); $future->resolvex(); @@ -773,14 +765,16 @@ EOTEXT } if ($this->shouldCommit()) { - $author_cmd = ''; - $author = $bundle->getAuthor(); - if ($author) { - $author_cmd = sprintf('-u %s ', $author); + $author = coalesce($bundle->getFullAuthor(), $bundle->getAuthorName()); + if ($author !== null) { + $author_cmd = csprintf('-u %s', $author); + } else { + $author_cmd = ''; } + $commit_message = $this->getCommitMessage($bundle); $future = $repository_api->execFutureLocal( - 'commit -A %C-l -', + 'commit -A %C -l -', $author_cmd); $future->write($commit_message); $future->resolvex();