diff --git a/src/land/engine/ArcanistGitLandEngine.php b/src/land/engine/ArcanistGitLandEngine.php --- a/src/land/engine/ArcanistGitLandEngine.php +++ b/src/land/engine/ArcanistGitLandEngine.php @@ -229,6 +229,8 @@ $api = $this->getRepositoryAPI(); $log = $this->getLogEngine(); + $this->confirmLegacyStrategyConfiguration(); + $is_empty = ($into_commit === null); if ($is_empty) { @@ -1429,4 +1431,50 @@ return $refspecs; } + private function confirmLegacyStrategyConfiguration() { + // TODO: See T13547. Remove this check in the future. This prevents users + // from accidentally executing a "squash" workflow under a configuration + // which would previously have executed a "merge" workflow. + + // We're fine if we have an explicit "--strategy". + if ($this->getStrategyArgument() !== null) { + return; + } + + // We're fine if we have an explicit "arc.land.strategy". + if ($this->getStrategyFromConfiguration() !== null) { + return; + } + + // We're fine if "history.immutable" is not set to "true". + $source_list = $this->getWorkflow()->getConfigurationSourceList(); + $config_list = $source_list->getStorageValueList('history.immutable'); + if (!$config_list) { + return; + } + + $config_value = (bool)last($config_list)->getValue(); + if (!$config_value) { + return; + } + + // We're in trouble: we would previously have selected "merge" and will + // now select "squash". Make sure the user knows what they're in for. + + echo tsprintf( + "\n%!\n%W\n\n", + pht('MERGE STRATEGY IS AMBIGUOUS'), + pht( + 'See <%s>. The default merge strategy under Git with '. + '"history.immutable" has changed from "merge" to "squash". Your '. + 'configuration is ambiguous under this behavioral change. '. + '(Use "--strategy" or configure "arc.land.strategy" to bypass '. + 'this check.)', + 'https://secure.phabricator.com/T13547')); + + throw new PhutilArgumentUsageException( + pht( + 'Desired merge strategy is ambiguous, choose an explicit strategy.')); + } + } diff --git a/src/land/engine/ArcanistLandEngine.php b/src/land/engine/ArcanistLandEngine.php --- a/src/land/engine/ArcanistLandEngine.php +++ b/src/land/engine/ArcanistLandEngine.php @@ -269,13 +269,17 @@ return $this->localState; } + final protected function getOntoConfigurationKey() { + return 'arc.land.onto'; + } + final protected function getOntoFromConfiguration() { $config_key = $this->getOntoConfigurationKey(); return $this->getWorkflow()->getConfig($config_key); } - final protected function getOntoConfigurationKey() { - return 'arc.land.onto'; + final protected function getOntoRemoteConfigurationKey() { + return 'arc.land.onto-remote'; } final protected function getOntoRemoteFromConfiguration() { @@ -283,8 +287,13 @@ return $this->getWorkflow()->getConfig($config_key); } - final protected function getOntoRemoteConfigurationKey() { - return 'arc.land.onto-remote'; + final protected function getStrategyConfigurationKey() { + return 'arc.land.strategy'; + } + + final protected function getStrategyFromConfiguration() { + $config_key = $this->getStrategyConfigurationKey(); + return $this->getWorkflow()->getConfig($config_key); } final protected function confirmRevisions(array $sets) { @@ -1457,8 +1466,7 @@ return $strategy; } - $strategy_key = 'arc.land.strategy'; - $strategy = $this->getWorkflow()->getConfig($strategy_key); + $strategy = $this->getStrategyFromConfiguration(); if ($strategy !== null) { if (!isset($supported_strategies[$strategy])) { throw new PhutilArgumentUsageException( @@ -1474,7 +1482,7 @@ pht( 'Merging with "%s" strategy, configured with "%s".', $strategy, - $strategy_key)); + $this->getStrategyConfigurationKey())); return $strategy; }