diff --git a/resources/sql/autopatches/20190412.herald.01.rebuild.php b/resources/sql/autopatches/20190412.herald.01.rebuild.php --- a/resources/sql/autopatches/20190412.herald.01.rebuild.php +++ b/resources/sql/autopatches/20190412.herald.01.rebuild.php @@ -1,3 +1,5 @@ key = $dict['key']; @@ -18,6 +22,7 @@ $this->name = $dict['name']; $this->after = $dict['after']; $this->dead = $dict['dead']; + $this->phase = $dict['phase']; } public function getLegacy() { @@ -44,6 +49,10 @@ return $this->key; } + public function getPhase() { + return $this->phase; + } + public function isDead() { return $this->dead; } @@ -52,4 +61,31 @@ return ($this->getType() == 'php'); } + public static function getPhaseList() { + return array_keys(self::getPhaseMap()); + } + + public static function getDefaultPhase() { + return self::PHASE_DEFAULT; + } + + private static function getPhaseMap() { + return array( + self::PHASE_DEFAULT => array( + 'order' => 0, + ), + self::PHASE_WORKER => array( + 'order' => 1, + ), + ); + } + + public function newSortVector() { + $map = self::getPhaseMap(); + $phase = $this->getPhase(); + + return id(new PhutilSortVector()) + ->addInt($map[$phase]['order']); + } + } diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php @@ -33,6 +33,7 @@ $table = id(new PhutilConsoleTable()) ->setShowHeader(false) ->addColumn('id', array('title' => pht('ID'))) + ->addColumn('phase', array('title' => pht('Phase'))) ->addColumn('host', array('title' => pht('Host'))) ->addColumn('status', array('title' => pht('Status'))) ->addColumn('duration', array('title' => pht('Duration'))) @@ -49,16 +50,22 @@ $duration = pht('%s us', new PhutilNumber($duration)); } - $table->addRow(array( - 'id' => $patch->getFullKey(), - 'host' => $ref->getRefKey(), - 'status' => in_array($patch->getFullKey(), $applied) - ? pht('Applied') - : pht('Not Applied'), - 'duration' => $duration, - 'type' => $patch->getType(), - 'name' => $patch->getName(), - )); + if (in_array($patch->getFullKey(), $applied)) { + $status = pht('Applied'); + } else { + $status = pht('Not Applied'); + } + + $table->addRow( + array( + 'id' => $patch->getFullKey(), + 'phase' => $patch->getPhase(), + 'host' => $ref->getRefKey(), + 'status' => $status, + 'duration' => $duration, + 'type' => $patch->getType(), + 'name' => $patch->getName(), + )); } $table->draw(); diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php @@ -922,6 +922,10 @@ $patches = $this->patches; $is_dryrun = $this->dryRun; + // We expect that patches should already be sorted properly. However, + // phase behavior will be wrong if they aren't, so make sure. + $patches = msortv($patches, 'newSortVector'); + $api_map = array(); foreach ($apis as $api) { $api_map[$api->getRef()->getRefKey()] = $api; diff --git a/src/infrastructure/storage/patch/PhabricatorSQLPatchList.php b/src/infrastructure/storage/patch/PhabricatorSQLPatchList.php --- a/src/infrastructure/storage/patch/PhabricatorSQLPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorSQLPatchList.php @@ -27,10 +27,20 @@ $directory)); } + $patch_type = $matches[1]; + $patch_full_path = rtrim($directory, '/').'/'.$patch; + + $attributes = array(); + if ($patch_type === 'php') { + $attributes = $this->getPHPPatchAttributes( + $patch, + $patch_full_path); + } + $patches[$patch] = array( - 'type' => $matches[1], - 'name' => rtrim($directory, '/').'/'.$patch, - ); + 'type' => $patch_type, + 'name' => $patch_full_path, + ) + $attributes; } return $patches; @@ -45,8 +55,16 @@ $specs = array(); $seen_namespaces = array(); + $phases = PhabricatorStoragePatch::getPhaseList(); + $phases = array_fuse($phases); + + $default_phase = PhabricatorStoragePatch::getDefaultPhase(); + foreach ($patch_lists as $patch_list) { - $last_key = null; + $last_keys = array_fill_keys( + array_keys($phases), + null); + foreach ($patch_list->getPatches() as $key => $patch) { if (!is_array($patch)) { throw new Exception( @@ -63,6 +81,7 @@ 'after' => true, 'legacy' => true, 'dead' => true, + 'phase' => true, ); foreach ($patch as $pkey => $pval) { @@ -128,8 +147,26 @@ $patch['legacy'] = false; } + if (!array_key_exists('phase', $patch)) { + $patch['phase'] = $default_phase; + } + + $patch_phase = $patch['phase']; + + if (!isset($phases[$patch_phase])) { + throw new Exception( + pht( + 'Storage patch "%s" specifies it should apply in phase "%s", '. + 'but this phase is unrecognized. Valid phases are: %s.', + $full_key, + $patch_phase, + implode(', ', array_keys($phases)))); + } + + $last_key = $last_keys[$patch_phase]; + if (!array_key_exists('after', $patch)) { - if ($last_key === null) { + if ($last_key === null && $patch_phase === $default_phase) { throw new Exception( pht( "Patch '%s' is missing key 'after', and is the first patch ". @@ -139,10 +176,14 @@ $full_key, get_class($patch_list))); } else { - $patch['after'] = array($last_key); + if ($last_key === null) { + $patch['after'] = array(); + } else { + $patch['after'] = array($last_key); + } } } - $last_key = $full_key; + $last_keys[$patch_phase] = $full_key; foreach ($patch['after'] as $after_key => $after) { if (strpos($after, ':') === false) { @@ -186,6 +227,21 @@ $key, $after)); } + + $patch_phase = $patch['phase']; + $after_phase = $specs[$after]['phase']; + + if ($patch_phase !== $after_phase) { + throw new Exception( + pht( + 'Storage patch "%s" executes in phase "%s", but depends on '. + 'patch "%s" which is in a different phase ("%s"). Patches '. + 'may not have dependencies across phases.', + $key, + $patch_phase, + $after, + $after_phase)); + } } } @@ -196,7 +252,94 @@ // TODO: Detect cycles? + $patches = msortv($patches, 'newSortVector'); + return $patches; } + private function getPHPPatchAttributes($patch_name, $full_path) { + $data = Filesystem::readFile($full_path); + + $phase_list = PhabricatorStoragePatch::getPhaseList(); + $phase_map = array_fuse($phase_list); + + $attributes = array(); + + $lines = phutil_split_lines($data, false); + foreach ($lines as $line) { + // Skip over the "PHP" line. + if (preg_match('(^<\?)', $line)) { + continue; + } + + // Skip over blank lines. + if (!strlen(trim($line))) { + continue; + } + + // If this is a "//" comment... + if (preg_match('(^\s*//)', $line)) { + $matches = null; + if (preg_match('(^\s*//\s*@(\S+)(?:\s+(.*))?\z)', $line, $matches)) { + $attr_key = $matches[1]; + $attr_value = trim(idx($matches, 2)); + + switch ($attr_key) { + case 'phase': + $phase_name = $attr_value; + + if (!strlen($phase_name)) { + throw new Exception( + pht( + 'Storage patch "%s" specifies a "@phase" attribute with '. + 'no phase value. Phase attributes must specify a value, '. + 'like "@phase default".', + $patch_name)); + } + + if (!isset($phase_map[$phase_name])) { + throw new Exception( + pht( + 'Storage patch "%s" specifies a "@phase" value ("%s"), '. + 'but this is not a recognized phase. Valid phases '. + 'are: %s.', + $patch_name, + $phase_name, + implode(', ', $phase_list))); + } + + if (isset($attributes['phase'])) { + throw new Exception( + pht( + 'Storage patch "%s" specifies a "@phase" value ("%s"), '. + 'but it already has a specified phase ("%s"). Patches '. + 'may not specify multiple phases.', + $patch_name, + $phase_name, + $attributes['phase'])); + } + + $attributes[$attr_key] = $phase_name; + break; + default: + throw new Exception( + pht( + 'Storage patch "%s" specifies attribute "%s", but this '. + 'attribute is unknown.', + $patch_name, + $attr_key)); + } + } + continue; + } + + // If this is anything else, we're all done. Attributes must be marked + // in the header of the file. + break; + } + + + return $attributes; + } + }