From 6604109f316ca93b74641f9af57cc9dbf8bb19b7 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 24 Jun 2016 17:45:13 +1200 Subject: [PATCH 1/5] API Split updater into a service and provide two default implementations: - GitMarkdownUpdater - SVNMarkdonUpdater Add logging for errors to SS_Log Secure escaping of shell arguments and error detection --- app/_config.php | 8 +- app/_config/config.yml | 6 +- app/code/RefreshMarkdownTask.php | 144 +++++++++++++++-------- app/code/updaters/GitMarkdownUpdater.php | 83 +++++++++++++ app/code/updaters/MarkdownUpdater.php | 12 ++ app/code/updaters/SVNMarkdownUpdater.php | 33 ++++++ 6 files changed, 234 insertions(+), 52 deletions(-) create mode 100644 app/code/updaters/GitMarkdownUpdater.php create mode 100644 app/code/updaters/MarkdownUpdater.php create mode 100644 app/code/updaters/SVNMarkdownUpdater.php diff --git a/app/_config.php b/app/_config.php index 120b348..fc9c99d 100644 --- a/app/_config.php +++ b/app/_config.php @@ -15,7 +15,7 @@ if(isset($_ENV['CLEARDB_DATABASE_URL'])) { global $databaseConfig; $parts = parse_url($_ENV['CLEARDB_DATABASE_URL']); - + $databaseConfig['type'] = 'MySQLDatabase'; $databaseConfig['server'] = $parts['host']; $databaseConfig['username'] = $parts['user']; @@ -52,6 +52,12 @@ DocumentationSearch::set_meta_data(array( 'Tags' => 'silverstripe sapphire php framework cms content management system' )); +// SS Platform logging +if(defined('AWS_SYSLOG_LEVEL')) { + $sysLogWriter = new SS_SysLogWriter('silverstripe', LOG_PID | LOG_CONS); + SS_Log::add_writer($sysLogWriter, (int)AWS_SYSLOG_LEVEL, '<='); +} + // Changelogs have heaps of phrases, but are rarely relevant for content searches Config::inst()->update('DocumentationSearch', 'boost_by_path', array( '/^changelog/' => 0.05 diff --git a/app/_config/config.yml b/app/_config/config.yml index 1e9964c..8baaffd 100644 --- a/app/_config/config.yml +++ b/app/_config/config.yml @@ -12,4 +12,8 @@ Controller: - ControllerExtension QuickFeedbackExtension: redirect_field: true - rate_limit: 1 \ No newline at end of file + rate_limit: 1 +Injector: + MarkdownUpdater: + class: GitMarkdownUpdater + diff --git a/app/code/RefreshMarkdownTask.php b/app/code/RefreshMarkdownTask.php index 49266ba..9182ca0 100644 --- a/app/code/RefreshMarkdownTask.php +++ b/app/code/RefreshMarkdownTask.php @@ -21,7 +21,7 @@ class RefreshMarkdownTask extends BuildTask /** * @var string */ - protected $description = "Downloads a fresh version of markdown documentation files from source"; + protected $description = "Downloads a fresh version of markdown documentation files from source. Options are force=1, addonly=1, and branch="; /** * @var bool @@ -35,10 +35,50 @@ class RefreshMarkdownTask extends BuildTask */ public function run($request) { - $this->request = $request; - $repositories = $this->getRepositories(); - foreach ($repositories as $repository) { - $this->cloneRepository($repository); + $this->request = $request; + $repositories = $this->getRepositories(); + $baseDir = $this->getSourceRoot(); + $updater = $this->getUpdater(); + + // Ensure root directory exists + if(!$this->ensureRootDirectory($baseDir)) { + return; + } + + // Update each repo + foreach ($repositories as list($repo, $folder, $branch)) { + $path = "{$baseDir}/{$folder}_{$branch}"; + + // Pass in ?force=1 to force delete existing repos, rather than trying to update them + if($this->request && $this->request->getVar('force') && file_exists($path)) { + $this->printLine("Removing {$path}"); + Filesystem::removeFolder($path); + } + + // Pass in ?addonly=1 to only update new branches + if($this->request && $this->request->getVar('addonly') && file_exists($path)) { + $this->printLine("Skipping update of {$branch}: Already exists at {$path};"); + continue; + } + + // Check if we want to update only a specific branch + if($this->request && ($onlyBranch = $this->request->getVar('branch')) && $onlyBranch != $branch) { + $this->printLine("Skipping update of {$branch}: doesn't match provided filter"); + continue; + } + + // Update this repo + $this->printLine("Beginning update of {$branch}"); + $errors = $updater->update($repo, $path, $branch); + + // Handle result + if(empty($errors)) { + $this->printLine("Successful update of {$branch}"); + } else { + foreach($errors as $error) { + $this->error($error); + } + } } $this->printLine(" "); $this->printLine("To re-index the freshly downloaded documentation files either:"); @@ -46,14 +86,22 @@ class RefreshMarkdownTask extends BuildTask $this->printLine("(2) point your browser at the url 'http://localhost/path/to/ssdocs/dev/tasks/RebuildLuceneDocsIndex?flush=1'"); } + /** + * Gets the service that will pull down remote markdown files + * @return MarkdownUpdater + */ + protected function getUpdater() { + return Injector::inst()->get('MarkdownUpdater'); + } + /** + * Get sources root directory + * * @return string - * - * @todo document this new configuration parameter */ - private function getPath() + private function getSourceRoot() { - return ASSETS_PATH; + return ASSETS_PATH . '/src'; } /** @@ -61,56 +109,52 @@ class RefreshMarkdownTask extends BuildTask */ private function printLine($message) { - $this->eol = Director::is_cli() ? PHP_EOL : "
"; - print $message . $this->eol; - flush(); + $eol = Director::is_cli() ? PHP_EOL : "
"; + echo $message . $eol; + ob_flush(); + flush(); } + /** + * Log an error + * + * @param string $message + */ + protected function error($message) + { + $this->printLine($message); + SS_Log::log($message, SS_Log::ERR); + } + /** * Returns the array of repos to source markdown docs from * * @return array */ - private function getRepositories() + protected function getRepositories() { - if($repos = $this->config()->documentation_repositories) - { - return $repos; - } else { - user_error("You need to set 'RefreshMarkdownTask:documentation_repositories' array in a yaml configuration file", E_USER_WARNING); - return null; - } + $repositories = $this->config()->documentation_repositories; + if(empty($repositories)) { + $this->error( + "You need to set 'RefreshMarkdownTask:documentation_repositories' array in a yaml configuration file" + ); + } + return $repositories; } - /** - * Clone $repository which contains the most current documentation source markdown files - * - * @param array $repository - */ - private function cloneRepository(array $repository) - { - - list($remote, $folder, $branch) = $repository; - - $path = $this->getPath(); - - exec("mkdir -p {$path}/src"); - exec("chmod -R 775 {$path}/src" ); - exec("rm -rf {$path}/src/{$folder}_{$branch}"); - chdir("{$path}/src"); - - // If the dev=1 flag is used when RefreshMarkdownTask is run, a full git clone of the framework repository is kept - // to enable local development of framework and doc.silverstripe.org from within doc.silverstripe.org. Otherwise, - // only the documentation source files are downloaded, only allowing the viewing of documentation files. - - if ($this->request instanceof SS_HTTPRequest && $this->request->getVar('dev')) { - $this->printLine("Cloning repository {$remote}/{$branch} into assets/src/{$folder}_{$branch}"); - exec("git clone --quiet https://github.com/{$remote}.git {$folder}_{$branch} --branch {$branch}"); - } else { - $this->printLine("Downloading the latest documentation files from repository {$remote}/{$branch} to assets/src/{$folder}_{$branch}"); - exec("svn export --quiet https://github.com/{$remote}.git/branches/{$branch}/docs {$folder}_{$branch}/docs"); - } - - } + /** + * Ensure root directory exists + * + * @param string $path + * @return bool True if the directory exists and is writable + */ + protected function ensureRootDirectory($path) { + Filesystem::makeFolder($path); + if(is_dir($path) && is_writable($path)) { + return true; + } + $this->error("Could not create {$path}"); + return false; + } } diff --git a/app/code/updaters/GitMarkdownUpdater.php b/app/code/updaters/GitMarkdownUpdater.php new file mode 100644 index 0000000..3817bbb --- /dev/null +++ b/app/code/updaters/GitMarkdownUpdater.php @@ -0,0 +1,83 @@ +runCommand('git --version', $errors)) { + return $errors; + } + + // Check if we should do a git update or re-clone + if (is_dir($path . '/.git')) { + return $this->doFetch($path, $branch); + } else { + return $this->doClone($repo, $path, $branch); + } + } + + /** + * Update existing git checkouts + * + * @param string $path + * @param string $branch + * @return array List of errors + */ + protected function doFetch($path, $branch) { + $errors = array(); + $checkoutCommand = sprintf( + 'cd %s && git checkout %s', + escapeshellarg($path), + escapeshellarg($branch) + ); + $pullCommand = sprintf( + 'cd %s && git pull origin %s', + escapeshellarg($path), + escapeshellarg($branch) + ); + + // Run + if($this->runCommand($checkoutCommand, $errors)) { + $this->runCommand($pullCommand, $errors); + } + return $errors; + } + + /** + * Clone a new git repo + * @param string $repo Repo slug + * @param string $path Destination path + * @param string $branch Branch name + * @return array + */ + protected function doClone($repo, $path, $branch) { + $errors = array(); + $remote = "https://github.com/{$repo}.git"; + $this->runCommand(sprintf( + "git clone --quiet %s %s --branch %s", + escapeshellarg($remote), + escapeshellarg($path), + escapeshellarg($branch) + ), $errors); + return $errors; + } + + /** + * Run this command + * + * @param string $cmd + * @param array $errors + * @return bool Flag if the command was successful + */ + protected function runCommand($cmd, &$errors = array()) { + exec($cmd, $output, $result); + if($result) { + $errors[] = "Error running command {$cmd}"; + return false; + } + return true; + } +} diff --git a/app/code/updaters/MarkdownUpdater.php b/app/code/updaters/MarkdownUpdater.php new file mode 100644 index 0000000..f5963f1 --- /dev/null +++ b/app/code/updaters/MarkdownUpdater.php @@ -0,0 +1,12 @@ +runCommand(sprintf( + "svn export %s %s", + escapeshellarg($svnPath), + escapeshellarg($svnDest) + ), $errors); + return $errors; + } + + /** + * Run this command + * + * @param string $cmd + * @param array $errors + * @return bool Flag if the command was successful + */ + protected function runCommand($cmd, &$errors = array()) { + exec($cmd, $output, $result); + if($result) { + $errors[] = "Error running command {$cmd}"; + return false; + } + return true; + } +} From 055888bf71040c34aa6ad1e3a3231d5cff8c9896 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 27 Jun 2016 17:09:20 +1200 Subject: [PATCH 2/5] force=1 only affects filtered branches --- app/code/RefreshMarkdownTask.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/code/RefreshMarkdownTask.php b/app/code/RefreshMarkdownTask.php index 9182ca0..18232ff 100644 --- a/app/code/RefreshMarkdownTask.php +++ b/app/code/RefreshMarkdownTask.php @@ -49,12 +49,6 @@ class RefreshMarkdownTask extends BuildTask foreach ($repositories as list($repo, $folder, $branch)) { $path = "{$baseDir}/{$folder}_{$branch}"; - // Pass in ?force=1 to force delete existing repos, rather than trying to update them - if($this->request && $this->request->getVar('force') && file_exists($path)) { - $this->printLine("Removing {$path}"); - Filesystem::removeFolder($path); - } - // Pass in ?addonly=1 to only update new branches if($this->request && $this->request->getVar('addonly') && file_exists($path)) { $this->printLine("Skipping update of {$branch}: Already exists at {$path};"); @@ -67,6 +61,12 @@ class RefreshMarkdownTask extends BuildTask continue; } + // Pass in ?force=1 to force delete existing repos, rather than trying to update them + if($this->request && $this->request->getVar('force') && file_exists($path)) { + $this->printLine("Removing {$path}"); + Filesystem::removeFolder($path); + } + // Update this repo $this->printLine("Beginning update of {$branch}"); $errors = $updater->update($repo, $path, $branch); From 2c8cba75be0bc4337145afaf1fcfb36f9488547d Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 27 Jun 2016 19:37:34 +1200 Subject: [PATCH 3/5] Use git reset --hard instead of git pull to refresh local files Better error reporting --- app/code/updaters/GitMarkdownUpdater.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/app/code/updaters/GitMarkdownUpdater.php b/app/code/updaters/GitMarkdownUpdater.php index 3817bbb..55c0eb9 100644 --- a/app/code/updaters/GitMarkdownUpdater.php +++ b/app/code/updaters/GitMarkdownUpdater.php @@ -28,20 +28,20 @@ class GitMarkdownUpdater implements MarkdownUpdater */ protected function doFetch($path, $branch) { $errors = array(); - $checkoutCommand = sprintf( - 'cd %s && git checkout %s', + $fetchCommand = sprintf( + 'cd %s && git fetch origin %s', escapeshellarg($path), escapeshellarg($branch) ); - $pullCommand = sprintf( - 'cd %s && git pull origin %s', + $resetCommand = sprintf( + 'cd %s && git reset --hard origin/%s', escapeshellarg($path), escapeshellarg($branch) ); // Run - if($this->runCommand($checkoutCommand, $errors)) { - $this->runCommand($pullCommand, $errors); + if($this->runCommand($fetchCommand, $errors)) { + $this->runCommand($resetCommand, $errors); } return $errors; } @@ -73,9 +73,12 @@ class GitMarkdownUpdater implements MarkdownUpdater * @return bool Flag if the command was successful */ protected function runCommand($cmd, &$errors = array()) { - exec($cmd, $output, $result); + exec("{$cmd} 2>&1 >/dev/null", $output, $result); if($result) { - $errors[] = "Error running command {$cmd}"; + $errors[] = "Error running command {$cmd}:"; + foreach($output as $error) { + $errors[] = ' * ' . $error; + } return false; } return true; From 415ed022e8e975828ecd4bc4dd8ae0c307ed25f6 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 27 Jun 2016 19:38:42 +1200 Subject: [PATCH 4/5] mark 3.4 as stable --- app/_config/docsviewer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/_config/docsviewer.yml b/app/_config/docsviewer.yml index 0ba0a35..c3db64c 100644 --- a/app/_config/docsviewer.yml +++ b/app/_config/docsviewer.yml @@ -20,7 +20,7 @@ DocumentationManifest: Title: "Framework" Version: "3.4" Branch: "3.4" - Stable: false + Stable: true DefaultEntity: true - Path: "assets/src/framework_3.3/docs/" From 93daffe4d16af39e1a0bce4069be7b6526ffae59 Mon Sep 17 00:00:00 2001 From: Cam Findlay Date: Mon, 4 Jul 2016 13:23:57 +1200 Subject: [PATCH 5/5] Set 3.4 stable --- app/_config/docsviewer.yml | 1 - app/code/RefreshMarkdownTask.php | 6 ++++- composer.json | 2 +- composer.lock | 40 ++++++++++++++++++-------------- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/app/_config/docsviewer.yml b/app/_config/docsviewer.yml index c3db64c..21e6197 100644 --- a/app/_config/docsviewer.yml +++ b/app/_config/docsviewer.yml @@ -27,7 +27,6 @@ DocumentationManifest: Title: "Framework" Version: "3.3" Branch: "3.3" - Stable: true DefaultEntity: true - Path: "assets/src/framework_3.2/docs/" diff --git a/app/code/RefreshMarkdownTask.php b/app/code/RefreshMarkdownTask.php index 18232ff..1deab14 100644 --- a/app/code/RefreshMarkdownTask.php +++ b/app/code/RefreshMarkdownTask.php @@ -111,8 +111,12 @@ class RefreshMarkdownTask extends BuildTask { $eol = Director::is_cli() ? PHP_EOL : "
"; echo $message . $eol; - ob_flush(); + if(ob_get_level() > 0) { + ob_flush(); + } flush(); + + } /** diff --git a/composer.json b/composer.json index 234f3fb..b21afca 100644 --- a/composer.json +++ b/composer.json @@ -5,7 +5,7 @@ "ext-gd": "*", "ext-mbstring": "*", "silverstripe/docsviewer": "^2.0", - "silverstripe/framework": "3.2.1", + "silverstripe/framework": "^3.2", "silverstripe/toolbar": "^4.0", "silverstripe/dynamodb": "^1.1", "mandrew/silverstripe-quickfeedback": "^0.2" diff --git a/composer.lock b/composer.lock index f26ea9f..f0920bd 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "7aca7c53bf2e2a013790143ca4493ffb", - "content-hash": "99728b10c87af2ab40e8f0da741ef8c0", + "hash": "cc3e0d21493519332387f87c4e668fb5", + "content-hash": "93abce3419beb70f39d06267aa1d5e84", "packages": [ { "name": "aws/aws-sdk-php", @@ -416,6 +416,7 @@ "rest", "web service" ], + "abandoned": "guzzlehttp/guzzle", "time": "2015-03-18 18:23:50" }, { @@ -633,16 +634,16 @@ }, { "name": "silverstripe/framework", - "version": "3.2.1", + "version": "3.4.0", "source": { "type": "git", "url": "https://github.com/silverstripe/silverstripe-framework.git", - "reference": "0178fa5a1873a0f1d6100268c86dab81a18c23a5" + "reference": "8008fcbe9769f1d2cb53c542f302ad6d32d185d6" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/silverstripe/silverstripe-framework/zipball/0178fa5a1873a0f1d6100268c86dab81a18c23a5", - "reference": "0178fa5a1873a0f1d6100268c86dab81a18c23a5", + "url": "https://api.github.com/repos/silverstripe/silverstripe-framework/zipball/8008fcbe9769f1d2cb53c542f302ad6d32d185d6", + "reference": "8008fcbe9769f1d2cb53c542f302ad6d32d185d6", "shasum": "" }, "require": { @@ -653,6 +654,11 @@ "phpunit/phpunit": "~3.7" }, "type": "silverstripe-module", + "extra": { + "branch-alias": { + "3.x-dev": "3.4.x-dev" + } + }, "autoload": { "classmap": [ "tests/behat/features/bootstrap" @@ -678,7 +684,7 @@ "framework", "silverstripe" ], - "time": "2015-11-16 03:17:10" + "time": "2016-06-02 22:59:29" }, { "name": "silverstripe/toolbar", @@ -726,16 +732,16 @@ }, { "name": "symfony/event-dispatcher", - "version": "v2.8.7", + "version": "v2.8.8", "source": { "type": "git", "url": "https://github.com/symfony/event-dispatcher.git", - "reference": "2a6b8713f8bdb582058cfda463527f195b066110" + "reference": "b180b70439dca70049b6b9b7e21d75e6e5d7aca9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/event-dispatcher/zipball/2a6b8713f8bdb582058cfda463527f195b066110", - "reference": "2a6b8713f8bdb582058cfda463527f195b066110", + "url": "https://api.github.com/repos/symfony/event-dispatcher/zipball/b180b70439dca70049b6b9b7e21d75e6e5d7aca9", + "reference": "b180b70439dca70049b6b9b7e21d75e6e5d7aca9", "shasum": "" }, "require": { @@ -782,7 +788,7 @@ ], "description": "Symfony EventDispatcher Component", "homepage": "https://symfony.com", - "time": "2016-06-06 11:11:27" + "time": "2016-06-29 05:29:29" }, { "name": "unclecheese/display-logic", @@ -1197,16 +1203,16 @@ }, { "name": "symfony/yaml", - "version": "v2.8.7", + "version": "v2.8.8", "source": { "type": "git", "url": "https://github.com/symfony/yaml.git", - "reference": "815fabf3f48c7d1df345a69d1ad1a88f59757b34" + "reference": "dba4bb5846798cd12f32e2d8f3f35d77045773c8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/yaml/zipball/815fabf3f48c7d1df345a69d1ad1a88f59757b34", - "reference": "815fabf3f48c7d1df345a69d1ad1a88f59757b34", + "url": "https://api.github.com/repos/symfony/yaml/zipball/dba4bb5846798cd12f32e2d8f3f35d77045773c8", + "reference": "dba4bb5846798cd12f32e2d8f3f35d77045773c8", "shasum": "" }, "require": { @@ -1242,7 +1248,7 @@ ], "description": "Symfony Yaml Component", "homepage": "https://symfony.com", - "time": "2016-06-06 11:11:27" + "time": "2016-06-29 05:29:29" } ], "aliases": [],