From 0b3cd39230e903009091ad0cc4ce539e405c215d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 8 Apr 2020 07:18:46 -0700 Subject: [PATCH] Update the "WorkingCopy" API and create a fallback "Filesystem" working copy Summary: Ref T11968. - Allow "WorkingCopy" objects to maintain an API object and update callers ("get...()" instead of "new...()"). - Always generate a WorkingCopy object and a RepositoryAPI object. Currently, code has to look like this: ``` $working_copy = ... if ($working_copy) { $repository_api = ... if ($repository_api [instanceof ... ]) { ``` This is clunky. There's also no reason some "arc" commands can't run outside a VCS working directory without special-casing how they interact with the filesystem. Conceptually, model the filesystem as a trivial VCS (which stores exactly one commit, always amends onto it, and discards history). Provide a trivial WorkingCopy and API for it. (This change isn't terribly interesting on its own, but chips away at landing the new Hardpoint infrastructure.) Test Plan: Ran `arc version`, `arc upgrade`. Maniphest Tasks: T11968 Differential Revision: https://secure.phabricator.com/D21070 --- src/__phutil_library_map__.php | 4 + src/repository/api/ArcanistFilesystemAPI.php | 100 ++++++++++++++++++ .../workflow/ArcanistVersionWorkflow.php | 11 +- src/workflow/ArcanistUpgradeWorkflow.php | 11 +- .../ArcanistFilesystemWorkingCopy.php | 27 +++++ src/workingcopy/ArcanistGitWorkingCopy.php | 2 +- .../ArcanistMercurialWorkingCopy.php | 2 +- .../ArcanistSubversionWorkingCopy.php | 2 +- src/workingcopy/ArcanistWorkingCopy.php | 34 +++++- 9 files changed, 170 insertions(+), 23 deletions(-) create mode 100644 src/repository/api/ArcanistFilesystemAPI.php create mode 100644 src/workingcopy/ArcanistFilesystemWorkingCopy.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f65edb89..0eb57b09 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -187,7 +187,9 @@ phutil_register_library_map(array( 'ArcanistFileUploader' => 'upload/ArcanistFileUploader.php', 'ArcanistFilenameLinter' => 'lint/linter/ArcanistFilenameLinter.php', 'ArcanistFilenameLinterTestCase' => 'lint/linter/__tests__/ArcanistFilenameLinterTestCase.php', + 'ArcanistFilesystemAPI' => 'repository/api/ArcanistFilesystemAPI.php', 'ArcanistFilesystemConfigurationSource' => 'config/source/ArcanistFilesystemConfigurationSource.php', + 'ArcanistFilesystemWorkingCopy' => 'workingcopy/ArcanistFilesystemWorkingCopy.php', 'ArcanistFlagWorkflow' => 'workflow/ArcanistFlagWorkflow.php', 'ArcanistFlake8Linter' => 'lint/linter/ArcanistFlake8Linter.php', 'ArcanistFlake8LinterTestCase' => 'lint/linter/__tests__/ArcanistFlake8LinterTestCase.php', @@ -1130,7 +1132,9 @@ phutil_register_library_map(array( 'ArcanistFileUploader' => 'Phobject', 'ArcanistFilenameLinter' => 'ArcanistLinter', 'ArcanistFilenameLinterTestCase' => 'ArcanistLinterTestCase', + 'ArcanistFilesystemAPI' => 'ArcanistRepositoryAPI', 'ArcanistFilesystemConfigurationSource' => 'ArcanistDictionaryConfigurationSource', + 'ArcanistFilesystemWorkingCopy' => 'ArcanistWorkingCopy', 'ArcanistFlagWorkflow' => 'ArcanistWorkflow', 'ArcanistFlake8Linter' => 'ArcanistExternalLinter', 'ArcanistFlake8LinterTestCase' => 'ArcanistExternalLinterTestCase', diff --git a/src/repository/api/ArcanistFilesystemAPI.php b/src/repository/api/ArcanistFilesystemAPI.php new file mode 100644 index 00000000..5626d29a --- /dev/null +++ b/src/repository/api/ArcanistFilesystemAPI.php @@ -0,0 +1,100 @@ + $root) { - $is_git = false; - $working_copy = ArcanistWorkingCopy::newFromWorkingDirectory($root); - if ($working_copy) { - $repository_api = $working_copy->newRepositoryAPI(); - if ($repository_api instanceof ArcanistGitAPI) { - $is_git = true; - } - } + + $repository_api = $working_copy->getRepositoryAPI(); + $is_git = ($repository_api instanceof ArcanistGitAPI); if (!$is_git) { throw new PhutilArgumentUsageException( diff --git a/src/workflow/ArcanistUpgradeWorkflow.php b/src/workflow/ArcanistUpgradeWorkflow.php index c1aa57cb..7759c10a 100644 --- a/src/workflow/ArcanistUpgradeWorkflow.php +++ b/src/workflow/ArcanistUpgradeWorkflow.php @@ -43,15 +43,10 @@ EOTEXT 'Preparing to upgrade "%s"...', $library)); - $is_git = false; - $working_copy = ArcanistWorkingCopy::newFromWorkingDirectory($root); - if ($working_copy) { - $repository_api = $working_copy->newRepositoryAPI(); - if ($repository_api instanceof ArcanistGitAPI) { - $is_git = true; - } - } + + $repository_api = $working_copy->getRepositoryAPI(); + $is_git = ($repository_api instanceof ArcanistGitAPI); if (!$is_git) { throw new PhutilArgumentUsageException( diff --git a/src/workingcopy/ArcanistFilesystemWorkingCopy.php b/src/workingcopy/ArcanistFilesystemWorkingCopy.php new file mode 100644 index 00000000..364c9cf5 --- /dev/null +++ b/src/workingcopy/ArcanistFilesystemWorkingCopy.php @@ -0,0 +1,27 @@ +getPath()); } diff --git a/src/workingcopy/ArcanistMercurialWorkingCopy.php b/src/workingcopy/ArcanistMercurialWorkingCopy.php index fde360c7..5c9bcde6 100644 --- a/src/workingcopy/ArcanistMercurialWorkingCopy.php +++ b/src/workingcopy/ArcanistMercurialWorkingCopy.php @@ -18,7 +18,7 @@ final class ArcanistMercurialWorkingCopy return new self(); } - public function newRepositoryAPI() { + protected function newRepositoryAPI() { return new ArcanistMercurialAPI($this->getPath()); } diff --git a/src/workingcopy/ArcanistSubversionWorkingCopy.php b/src/workingcopy/ArcanistSubversionWorkingCopy.php index c368dfe5..e786c729 100644 --- a/src/workingcopy/ArcanistSubversionWorkingCopy.php +++ b/src/workingcopy/ArcanistSubversionWorkingCopy.php @@ -70,7 +70,7 @@ final class ArcanistSubversionWorkingCopy return head($candidates); } - public function newRepositoryAPI() { + protected function newRepositoryAPI() { return new ArcanistSubversionAPI($this->getPath()); } diff --git a/src/workingcopy/ArcanistWorkingCopy.php b/src/workingcopy/ArcanistWorkingCopy.php index 3a6a8ced..f0687f6e 100644 --- a/src/workingcopy/ArcanistWorkingCopy.php +++ b/src/workingcopy/ArcanistWorkingCopy.php @@ -5,6 +5,7 @@ abstract class ArcanistWorkingCopy private $path; private $workingDirectory; + private $repositoryAPI; public static function newFromWorkingDirectory($path) { $working_types = id(new PhutilClassMapQuery()) @@ -25,8 +26,7 @@ abstract class ArcanistWorkingCopy continue; } - $working_copy->path = $ancestor_path; - $working_copy->workingDirectory = $path; + self::configureWorkingCopy($working_copy, $ancestor_path, $path); $candidates[] = $working_copy; } @@ -50,7 +50,16 @@ abstract class ArcanistWorkingCopy return $deepest->selectFromNestedWorkingCopies($candidates); } - return null; + // If we haven't found a legitimate working copy that belongs to a + // supported version control system, return a "filesystem" working copy. + // This allows some commands to work as expected even if run outside + // of a real working copy. + + $working_copy = new ArcanistFilesystemWorkingCopy(); + + self::configureWorkingCopy($working_copy, $ancestor_path, $path); + + return $working_copy; } abstract protected function newWorkingCopyFromDirectories( @@ -110,6 +119,23 @@ abstract class ArcanistWorkingCopy return last($candidates); } - abstract public function newRepositoryAPI(); + final public function getRepositoryAPI() { + if (!$this->repositoryAPI) { + $this->repositoryAPI = $this->newRepositoryAPI(); + } + + return $this->repositoryAPI; + } + + abstract protected function newRepositoryAPI(); + + private static function configureWorkingCopy( + ArcanistWorkingCopy $working_copy, + $ancestor_path, + $path) { + + $working_copy->path = $ancestor_path; + $working_copy->workingDirectory = $path; + } }