Page MenuHomePhabricator
Files F15421449

D21332.id.diff
No OneTemporary
Actions

D21332.id.diff
View Options

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;
}

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 22, 11:55 PM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7383050
Default Alt Text
D21332.id.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility