From 55d9cc70131678d1d373bb6605692cc2be475772 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Sep 2015 12:44:59 -0700 Subject: [PATCH 1/4] Improve temporary file upload API and add viewPolicy support Summary: Ref T7148. In D14056, I let `arc upload` upload temporary files, but this is a better way to do some of the internals. Also add support for setting a `viewPolicy`. Test Plan: Used `arc upload`, `arc upload --temporary`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7148 Differential Revision: https://secure.phabricator.com/D14075 --- src/upload/ArcanistFileDataRef.php | 44 +++++++++++++++++++++++++ src/upload/ArcanistFileUploader.php | 37 ++++----------------- src/workflow/ArcanistUploadWorkflow.php | 9 +++-- 3 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/upload/ArcanistFileDataRef.php b/src/upload/ArcanistFileDataRef.php index 795fbd67..318418ec 100644 --- a/src/upload/ArcanistFileDataRef.php +++ b/src/upload/ArcanistFileDataRef.php @@ -27,6 +27,8 @@ final class ArcanistFileDataRef extends Phobject { private $errors = array(); private $phid; private $fileHandle; + private $deleteAfterEpoch; + private $viewPolicy; /* -( Configuring File References )---------------------------------------- */ @@ -107,6 +109,48 @@ final class ArcanistFileDataRef extends Phobject { } + /** + * @task config + */ + public function setViewPolicy($view_policy) { + $this->viewPolicy = $view_policy; + return $this; + } + + + /** + * @task config + */ + public function getViewPolicy() { + return $this->viewPolicy; + } + + + /** + * Configure a file to be temporary instead of permanent. + * + * By default, files are retained indefinitely until explicitly deleted. If + * you want to upload a temporary file instead, you can specify an epoch + * timestamp. The file will be deleted after this time. + * + * @param int Epoch timestamp to retain the file until. + * @return this + * @task config + */ + public function setDeleteAfterEpoch($epoch) { + $this->deleteAfterEpoch = $epoch; + return $this; + } + + + /** + * @task config + */ + public function getDeleteAfterEpoch() { + return $this->deleteAfterEpoch; + } + + /* -( Handling Upload Results )-------------------------------------------- */ diff --git a/src/upload/ArcanistFileUploader.php b/src/upload/ArcanistFileUploader.php index ba960775..27ec8d34 100644 --- a/src/upload/ArcanistFileUploader.php +++ b/src/upload/ArcanistFileUploader.php @@ -27,7 +27,6 @@ final class ArcanistFileUploader extends Phobject { private $conduit; private $files; - private $config = array(); /* -( Configuring the Uploader )------------------------------------------- */ @@ -79,33 +78,6 @@ final class ArcanistFileUploader extends Phobject { } - /** - * Configure a file to be temporary instead of permanent. - * - * By default, files are retained indefinitely until explicitly deleted. If - * you want to upload a temporary file instead, you can specify an epoch - * timestamp. The file will be deleted after this time. - * - * @param string Key identifying the file you want to make temporary, as - * passed to @{method:addFile}. - * @param int Epoch timestamp to retain the file until. - * @return this - * @task add - */ - public function setDeleteFileAfterEpoch($file_key, $epoch) { - if (empty($this->files[$file_key])) { - throw new Exception( - pht( - 'No file with given key ("%s") has been added to this uploader.', - $file_key)); - } - - $this->config[$file_key]['deleteAfterEpoch'] = $epoch; - - return $this; - } - - /* -( Uploading Files )---------------------------------------------------- */ @@ -144,19 +116,22 @@ final class ArcanistFileUploader extends Phobject { $conduit = $this->conduit; $futures = array(); foreach ($files as $key => $file) { - $config = idx($this->config, $key, array()); - $params = array( 'name' => $file->getName(), 'contentLength' => $file->getByteSize(), 'contentHash' => $file->getContentHash(), ); - $delete_after = idx($config, 'deleteAfterEpoch'); + $delete_after = $file->getDeleteAfterEpoch(); if ($delete_after !== null) { $params['deleteAfterEpoch'] = $delete_after; } + $view_policy = $file->getViewPolicy(); + if ($view_policy !== null) { + $params['viewPolicy'] = $view_policy; + } + $futures[$key] = $conduit->callMethod('file.allocate', $params); } diff --git a/src/workflow/ArcanistUploadWorkflow.php b/src/workflow/ArcanistUploadWorkflow.php index b4414a39..a31ce0f0 100644 --- a/src/workflow/ArcanistUploadWorkflow.php +++ b/src/workflow/ArcanistUploadWorkflow.php @@ -69,13 +69,12 @@ EOTEXT ->setName(basename($path)) ->setPath($path); - $uploader->addFile($file, $key); - if ($is_temporary) { - $uploader->setDeleteFileAfterEpoch( - $key, - time() + phutil_units('24 hours in seconds')); + $expires_at = time() + phutil_units('24 hours in seconds'); + $file->setDeleteAfterEpoch($expires_at); } + + $uploader->addFile($file); } $files = $uploader->uploadFiles(); From 8f3a002cdb1907fc0fe8104ca9f7c2dc067e94e5 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Mon, 7 Sep 2015 13:44:47 -0700 Subject: [PATCH 2/4] Allow upgrading in branch `stable` Summary: close T9043. This still allows for one dir to be in `master` and the other in `stable`, but hopefully that's not going to be a problem. Test Plan: Clone arc/libph, checkout `stable`, run arc upgrade. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: cburroughs, epriestley Maniphest Tasks: T9043 Differential Revision: https://secure.phabricator.com/D14076 --- src/workflow/ArcanistUpgradeWorkflow.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/workflow/ArcanistUpgradeWorkflow.php b/src/workflow/ArcanistUpgradeWorkflow.php index ed30593f..c9e0e858 100644 --- a/src/workflow/ArcanistUpgradeWorkflow.php +++ b/src/workflow/ArcanistUpgradeWorkflow.php @@ -53,18 +53,18 @@ EOTEXT $this->setRepositoryAPI($repository); - // Require no local changes. $this->requireCleanWorkingCopy(); - // Require the library be on master. $branch_name = $repository->getBranchName(); - if ($branch_name != 'master') { + if ($branch_name != 'master' && $branch_name != 'stable') { throw new ArcanistUsageException( pht( - "%s must be on branch '%s' to be automatically upgraded. ". + "%s must be on either branch '%s' or '%s' to be automatically ". + "upgraded. ". "This copy of %s (in '%s') is on branch '%s'.", $lib, 'master', + 'stable', $lib, $root, $branch_name)); From bdab422440f5d7417403d23b770d9c2131a45c70 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Tue, 8 Sep 2015 10:39:26 -0700 Subject: [PATCH 3/4] arc linters: switch `name` and `short` Summary: I think they were wrong. Test Plan: `arc linters nolint`. It would fail without it. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, #lint Differential Revision: https://secure.phabricator.com/D14084 --- src/workflow/ArcanistLintersWorkflow.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/workflow/ArcanistLintersWorkflow.php b/src/workflow/ArcanistLintersWorkflow.php index a521a586..3c7d5606 100644 --- a/src/workflow/ArcanistLintersWorkflow.php +++ b/src/workflow/ArcanistLintersWorkflow.php @@ -104,8 +104,8 @@ EOTEXT $console->writeOut( "** %s ** **%s** (%s)\n", $text, - nonempty($linter['short'], '-'), - $linter['name']); + nonempty($linter['name'], '-'), + $linter['short']); if ($linter['exception']) { $console->writeOut( @@ -241,11 +241,11 @@ EOTEXT } $linter_info[$key] = array( - 'short' => $linter->getLinterConfigurationName(), + 'name' => $linter->getLinterConfigurationName(), 'class' => get_class($linter), 'status' => $status, 'version' => $version, - 'name' => $linter->getInfoName(), + 'short' => $linter->getInfoName(), 'uri' => $linter->getInfoURI(), 'description' => $linter->getInfoDescription(), 'exception' => $exception, From 28b89785fe5f933a4002434e5b9b3f91f650d2c1 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Tue, 8 Sep 2015 14:52:43 -0700 Subject: [PATCH 4/4] update linter short desc Summary: ref T9353. Test Plan: arc linters Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: joshuaspence, Korvin Maniphest Tasks: T9353 Differential Revision: https://secure.phabricator.com/D14085 --- src/lint/linter/ArcanistComposerLinter.php | 2 +- src/lint/linter/ArcanistCppcheckLinter.php | 2 +- src/lint/linter/ArcanistCpplintLinter.php | 2 +- src/lint/linter/ArcanistFlake8Linter.php | 2 +- src/lint/linter/ArcanistHLintLinter.php | 2 +- src/lint/linter/ArcanistJSHintLinter.php | 2 +- src/lint/linter/ArcanistJscsLinter.php | 2 +- src/lint/linter/ArcanistPyFlakesLinter.php | 2 +- src/lint/linter/ArcanistRuboCopLinter.php | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/lint/linter/ArcanistComposerLinter.php b/src/lint/linter/ArcanistComposerLinter.php index 02b7c673..a9eded3e 100644 --- a/src/lint/linter/ArcanistComposerLinter.php +++ b/src/lint/linter/ArcanistComposerLinter.php @@ -5,7 +5,7 @@ final class ArcanistComposerLinter extends ArcanistLinter { const LINT_OUT_OF_DATE = 1; public function getInfoName() { - return pht('Composer'); + return pht('Composer Dependency Manager'); } public function getInfoDescription() { diff --git a/src/lint/linter/ArcanistCppcheckLinter.php b/src/lint/linter/ArcanistCppcheckLinter.php index fe9b578f..7c063956 100644 --- a/src/lint/linter/ArcanistCppcheckLinter.php +++ b/src/lint/linter/ArcanistCppcheckLinter.php @@ -6,7 +6,7 @@ final class ArcanistCppcheckLinter extends ArcanistExternalLinter { public function getInfoName() { - return 'Cppcheck'; + return 'C++ linter'; } public function getInfoURI() { diff --git a/src/lint/linter/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index 0680d474..07200af1 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -6,7 +6,7 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { public function getLinterName() { - return 'CPPLINT'; + return 'C++ Google\'s Styleguide'; } public function getLinterConfigurationName() { diff --git a/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php index db8e9177..bd45082d 100644 --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -7,7 +7,7 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter { public function getInfoName() { - return 'Flake8'; + return 'Python Flake8 multi-linter'; } public function getInfoURI() { diff --git a/src/lint/linter/ArcanistHLintLinter.php b/src/lint/linter/ArcanistHLintLinter.php index 5a197ce4..f8c1df89 100644 --- a/src/lint/linter/ArcanistHLintLinter.php +++ b/src/lint/linter/ArcanistHLintLinter.php @@ -6,7 +6,7 @@ final class ArcanistHLintLinter extends ArcanistExternalLinter { public function getInfoName() { - return 'HLint'; + return 'Haskell Linter'; } public function getInfoURI() { diff --git a/src/lint/linter/ArcanistJSHintLinter.php b/src/lint/linter/ArcanistJSHintLinter.php index 1f9a766d..9d485bad 100644 --- a/src/lint/linter/ArcanistJSHintLinter.php +++ b/src/lint/linter/ArcanistJSHintLinter.php @@ -9,7 +9,7 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter { private $jshintrc; public function getInfoName() { - return 'JSHint'; + return 'JavaScript error checking'; } public function getInfoURI() { diff --git a/src/lint/linter/ArcanistJscsLinter.php b/src/lint/linter/ArcanistJscsLinter.php index d1c0b3c4..338e08d3 100644 --- a/src/lint/linter/ArcanistJscsLinter.php +++ b/src/lint/linter/ArcanistJscsLinter.php @@ -6,7 +6,7 @@ final class ArcanistJscsLinter extends ArcanistExternalLinter { private $preset; public function getInfoName() { - return 'JSCS'; + return 'JavaScript Code Style'; } public function getInfoURI() { diff --git a/src/lint/linter/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php index 3a79e02e..c312487c 100644 --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -10,7 +10,7 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter { } public function getInfoName() { - return pht('PyFlakes'); + return pht('Python PyFlakes'); } public function getInfoDescription() { diff --git a/src/lint/linter/ArcanistRuboCopLinter.php b/src/lint/linter/ArcanistRuboCopLinter.php index c7585ca6..812a0159 100644 --- a/src/lint/linter/ArcanistRuboCopLinter.php +++ b/src/lint/linter/ArcanistRuboCopLinter.php @@ -5,7 +5,7 @@ final class ArcanistRuboCopLinter extends ArcanistExternalLinter { private $config; public function getInfoName() { - return 'RuboCop'; + return 'Ruby static code analyzer'; } public function getInfoURI() {