Provide modern config options for "arc land" configuration

Summary: Ref T13546. Adds modern support for "arc.land.onto", "arc.land.onto-remote", and "history.immutable".

Test Plan: Read configuration in a future change.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21313

src/__phutil_library_map__.php

@@ -50,6 +50,7 @@ phutil_register_library_map(array(
     'ArcanistBlacklistedFunctionXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBlacklistedFunctionXHPASTLinterRuleTestCase.php',
     'ArcanistBlindlyTrustHTTPEngineExtension' => 'configuration/ArcanistBlindlyTrustHTTPEngineExtension.php',
     'ArcanistBookmarkWorkflow' => 'workflow/ArcanistBookmarkWorkflow.php',
+    'ArcanistBoolConfigOption' => 'config/option/ArcanistBoolConfigOption.php',
     'ArcanistBraceFormattingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php',
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php',
     'ArcanistBranchRef' => 'ref/ArcanistBranchRef.php',
@@ -328,6 +329,7 @@ phutil_register_library_map(array(
     'ArcanistMissingLinterException' => 'lint/linter/exception/ArcanistMissingLinterException.php',
     'ArcanistModifierOrderingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistModifierOrderingXHPASTLinterRule.php',
     'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistModifierOrderingXHPASTLinterRuleTestCase.php',
+    'ArcanistMultiSourceConfigOption' => 'config/option/ArcanistMultiSourceConfigOption.php',
     'ArcanistNamespaceFirstStatementXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNamespaceFirstStatementXHPASTLinterRule.php',
     'ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase.php',
     'ArcanistNamingConventionsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistNamingConventionsXHPASTLinterRule.php',
@@ -420,7 +422,6 @@ phutil_register_library_map(array(
     'ArcanistRuntime' => 'runtime/ArcanistRuntime.php',
     'ArcanistRuntimeConfigurationSource' => 'config/source/ArcanistRuntimeConfigurationSource.php',
     'ArcanistRuntimeHardpointQuery' => 'toolset/query/ArcanistRuntimeHardpointQuery.php',
-    'ArcanistScalarConfigOption' => 'config/option/ArcanistScalarConfigOption.php',
     'ArcanistScalarHardpoint' => 'hardpoint/ArcanistScalarHardpoint.php',
     'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php',
     'ArcanistSelfClassReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSelfClassReferenceXHPASTLinterRule.php',
@@ -437,6 +438,7 @@ phutil_register_library_map(array(
     'ArcanistSimpleSymbolRef' => 'ref/simple/ArcanistSimpleSymbolRef.php',
     'ArcanistSimpleSymbolRefInspector' => 'ref/simple/ArcanistSimpleSymbolRefInspector.php',
     'ArcanistSingleLintEngine' => 'lint/engine/ArcanistSingleLintEngine.php',
+    'ArcanistSingleSourceConfigOption' => 'config/option/ArcanistSingleSourceConfigOption.php',
     'ArcanistSlownessXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php',
     'ArcanistSlownessXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistSlownessXHPASTLinterRuleTestCase.php',
     'ArcanistSpellingLinter' => 'lint/linter/ArcanistSpellingLinter.php',
@@ -444,6 +446,7 @@ phutil_register_library_map(array(
     'ArcanistStaticThisXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistStaticThisXHPASTLinterRule.php',
     'ArcanistStaticThisXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistStaticThisXHPASTLinterRuleTestCase.php',
     'ArcanistStringConfigOption' => 'config/option/ArcanistStringConfigOption.php',
+    'ArcanistStringListConfigOption' => 'config/option/ArcanistStringListConfigOption.php',
     'ArcanistSubversionAPI' => 'repository/api/ArcanistSubversionAPI.php',
     'ArcanistSubversionWorkingCopy' => 'workingcopy/ArcanistSubversionWorkingCopy.php',
     'ArcanistSummaryLintRenderer' => 'lint/renderer/ArcanistSummaryLintRenderer.php',
@@ -1016,7 +1019,7 @@ phutil_register_library_map(array(
     'ArcanistAliasFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistAliasFunctionXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistAliasWorkflow' => 'ArcanistWorkflow',
-    'ArcanistAliasesConfigOption' => 'ArcanistListConfigOption',
+    'ArcanistAliasesConfigOption' => 'ArcanistMultiSourceConfigOption',
     'ArcanistAmendWorkflow' => 'ArcanistArcWorkflow',
     'ArcanistAnoidWorkflow' => 'ArcanistArcWorkflow',
     'ArcanistArcConfigurationEngineExtension' => 'ArcanistConfigurationEngineExtension',
@@ -1041,6 +1044,7 @@ phutil_register_library_map(array(
     'ArcanistBlacklistedFunctionXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistBlindlyTrustHTTPEngineExtension' => 'PhutilHTTPEngineExtension',
     'ArcanistBookmarkWorkflow' => 'ArcanistFeatureBaseWorkflow',
+    'ArcanistBoolConfigOption' => 'ArcanistSingleSourceConfigOption',
     'ArcanistBraceFormattingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistBranchRef' => 'ArcanistRef',
@@ -1313,7 +1317,7 @@ phutil_register_library_map(array(
     'ArcanistLintersWorkflow' => 'ArcanistWorkflow',
     'ArcanistListAssignmentXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistListAssignmentXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
-    'ArcanistListConfigOption' => 'ArcanistConfigOption',
+    'ArcanistListConfigOption' => 'ArcanistSingleSourceConfigOption',
     'ArcanistListWorkflow' => 'ArcanistWorkflow',
     'ArcanistLocalConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
     'ArcanistLogEngine' => 'Phobject',
@@ -1333,6 +1337,7 @@ phutil_register_library_map(array(
     'ArcanistMissingLinterException' => 'Exception',
     'ArcanistModifierOrderingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+    'ArcanistMultiSourceConfigOption' => 'ArcanistConfigOption',
     'ArcanistNamespaceFirstStatementXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistNamespaceFirstStatementXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistNamingConventionsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -1430,7 +1435,6 @@ phutil_register_library_map(array(
     'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistRuntimeConfigurationSource' => 'ArcanistDictionaryConfigurationSource',
     'ArcanistRuntimeHardpointQuery' => 'ArcanistHardpointQuery',
-    'ArcanistScalarConfigOption' => 'ArcanistConfigOption',
     'ArcanistScalarHardpoint' => 'ArcanistHardpoint',
     'ArcanistScriptAndRegexLinter' => 'ArcanistLinter',
     'ArcanistSelfClassReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -1447,13 +1451,15 @@ phutil_register_library_map(array(
     'ArcanistSimpleSymbolRef' => 'ArcanistSymbolRef',
     'ArcanistSimpleSymbolRefInspector' => 'ArcanistRefInspector',
     'ArcanistSingleLintEngine' => 'ArcanistLintEngine',
+    'ArcanistSingleSourceConfigOption' => 'ArcanistConfigOption',
     'ArcanistSlownessXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistSlownessXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistSpellingLinter' => 'ArcanistLinter',
     'ArcanistSpellingLinterTestCase' => 'ArcanistLinterTestCase',
     'ArcanistStaticThisXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistStaticThisXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
-    'ArcanistStringConfigOption' => 'ArcanistScalarConfigOption',
+    'ArcanistStringConfigOption' => 'ArcanistSingleSourceConfigOption',
+    'ArcanistStringListConfigOption' => 'ArcanistListConfigOption',
     'ArcanistSubversionAPI' => 'ArcanistRepositoryAPI',
     'ArcanistSubversionWorkingCopy' => 'ArcanistWorkingCopy',
     'ArcanistSummaryLintRenderer' => 'ArcanistLintRenderer',

src/config/arc/ArcanistArcConfigurationEngineExtension.php

@@ -29,38 +29,6 @@ final class ArcanistArcConfigurationEngineExtension
           'off of.'),
         'example' => '"develop"',
       ),
-      'arc.land.onto.default' => array(
-        'type' => 'string',
-        'help' => pht(
-          'The name of the default branch to land changes onto when '.
-          '`%s` is run.',
-          'arc land'),
-        'example' => '"develop"',
-      ),
-
-      'arc.autostash' => array(
-        'type' => 'bool',
-        'help' => pht(
-          'Whether %s should permit the automatic stashing of changes in the '.
-          'working directory when requiring a clean working copy. This option '.
-          'should only be used when users understand how to restore their '.
-          'working directory from the local stash if an Arcanist operation '.
-          'causes an unrecoverable error.',
-          'arc'),
-        'default' => false,
-        'example' => 'false',
-      ),
-
-      'history.immutable' => array(
-        'type' => 'bool',
-        'legacy' => 'immutable_history',
-        'help' => pht(
-          'If true, %s will never change repository history (e.g., through '.
-          'amending or rebasing). Defaults to true in Mercurial and false in '.
-          'Git. This setting has no effect in Subversion.',
-          'arc'),
-        'example' => 'false',
-      ),
       'editor' => array(
         'type' => 'string',
         'help' => pht(
@@ -111,9 +79,9 @@ final class ArcanistArcConfigurationEngineExtension
         ->setHelp(
           pht(
             'Associate the working copy with a specific Phabricator '.
-            'repository. Normally, `arc` can figure this association out on '.
-            'its own, but if your setup is unusual you can use this option '.
-            'to tell it what the desired value is.'))
+            'repository. Normally, Arcanist can figure this association '.
+            'out on its own, but if your setup is unusual you can use '.
+            'this option to tell it what the desired value is.'))
         ->setExamples(
           array(
             'libexample',
@@ -145,6 +113,42 @@ final class ArcanistArcConfigurationEngineExtension
           pht(
             'Configured command aliases. Use the "alias" workflow to define '.
             'aliases.')),
+      id(new ArcanistStringListConfigOption())
+        ->setKey('arc.land.onto')
+        ->setDefaultValue(array())
+        ->setSummary(pht('Default list of "onto" refs for "arc land".'))
+        ->setHelp(
+          pht(
+            'Specifies the default behavior when "arc land" is run with '.
+            'no "--onto" flag.'))
+        ->setExamples(
+          array(
+            '["master"]',
+          )),
+      id(new ArcanistStringConfigOption())
+        ->setKey('arc.land.onto-remote')
+        ->setSummary(pht('Default list of "onto" remote for "arc land".'))
+        ->setHelp(
+          pht(
+            'Specifies the default behavior when "arc land" is run with '.
+            'no "--onto-remote" flag.'))
+        ->setExamples(
+          array(
+            'origin',
+          )),
+      id(new ArcanistBoolConfigOption())
+        ->setKey('history.immutable')
+        ->setSummary(
+          pht(
+            'Configure use of history mutation operations like amends '.
+            'and rebases.'))
+        ->setHelp(
+          pht(
+            'If this option is set to "true", Arcanist will treat the '.
+            'repository history as immutable and will never issue '.
+            'commands which rewrite repository history (like amends or '.
+            'rebases). This option defaults to "true" in Mercurial, '.
+            '"false" in Git, and has no effect in Subversion.')),
     );
   }
 

src/config/option/ArcanistAliasesConfigOption.php

@@ -1,7 +1,7 @@
 <?php
 
 final class ArcanistAliasesConfigOption
-  extends ArcanistListConfigOption {
+  extends ArcanistMultiSourceConfigOption {
 
   public function getType() {
     return 'list<alias>';

src/config/option/ArcanistBoolConfigOption.php

@@ -0,0 +1,35 @@
+<?php
+
+final class ArcanistBoolConfigOption
+  extends ArcanistSingleSourceConfigOption {
+
+  public function getType() {
+    return 'bool';
+  }
+
+  public function getStorageValueFromStringValue($value) {
+    if ($value === 'true') {
+      return true;
+    }
+
+    if ($value === 'false') {
+      return false;
+    }
+
+    throw new PhutilArgumentUsageException(
+      pht('Specify either "true" or "false".'));
+  }
+
+  public function getDisplayValueFromValue($value) {
+    if ($value) {
+      return 'true';
+    } else {
+      return 'false';
+    }
+  }
+
+  public function getStorageValueFromValue($value) {
+    return $value;
+  }
+
+}

src/config/option/ArcanistListConfigOption.php

@@ -1,32 +1,58 @@
 <?php
 
 abstract class ArcanistListConfigOption
-  extends ArcanistConfigOption {
+  extends ArcanistSingleSourceConfigOption {
 
-  public function getValueFromStorageValueList(array $list) {
-    assert_instances_of($list, 'ArcanistConfigurationSourceValue');
-
-    $result_list = array();
-    foreach ($list as $source_value) {
-      $source = $source_value->getConfigurationSource();
-      $storage_value = $this->getStorageValueFromSourceValue($source_value);
-
-      $items = $this->getValueFromStorageValue($storage_value);
-      foreach ($items as $item) {
-        $result_list[] = new ArcanistConfigurationSourceValue(
-          $source,
-          $item);
-      }
+  final public function getStorageValueFromStringValue($value) {
+    try {
+      $json_value = phutil_json_decode($value);
+    } catch (PhutilJSONParserException $ex) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Value "%s" is not valid, specify a JSON list: %s',
+          $value,
+          $ex->getMessage()));
     }
 
-    $result_list = $this->didReadStorageValueList($result_list);
-
-    return $result_list;
+    if (!is_array($json_value) || !phutil_is_natural_list($json_value)) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Value "%s" is not valid: expected a list, got "%s".',
+          $value,
+          phutil_describe_type($json_value)));
     }
 
-  protected function didReadStorageValueList(array $list) {
-    assert_instances_of($list, 'ArcanistConfigurationSourceValue');
-    return mpull($list, 'getValue');
+    foreach ($json_value as $idx => $item) {
+      $this->validateListItem($idx, $item);
     }
 
+    return $json_value;
+  }
+
+  final public function getValueFromStorageValue($value) {
+    if (!is_array($value)) {
+      throw new Exception(pht('Expected a list!'));
+    }
+
+    if (!phutil_is_natural_list($value)) {
+      throw new Exception(pht('Expected a natural list!'));
+    }
+
+    foreach ($value as $idx => $item) {
+      $this->validateListItem($idx, $item);
+    }
+
+    return $value;
+  }
+
+  public function getDisplayValueFromValue($value) {
+    return json_encode($value);
+  }
+
+  public function getStorageValueFromValue($value) {
+    return $value;
+  }
+
+  abstract protected function validateListItem($idx, $item);
+
 }

src/config/option/ArcanistMultiSourceConfigOption.php

@@ -0,0 +1,32 @@
+<?php
+
+abstract class ArcanistMultiSourceConfigOption
+  extends ArcanistConfigOption {
+
+  public function getValueFromStorageValueList(array $list) {
+    assert_instances_of($list, 'ArcanistConfigurationSourceValue');
+
+    $result_list = array();
+    foreach ($list as $source_value) {
+      $source = $source_value->getConfigurationSource();
+      $storage_value = $this->getStorageValueFromSourceValue($source_value);
+
+      $items = $this->getValueFromStorageValue($storage_value);
+      foreach ($items as $item) {
+        $result_list[] = new ArcanistConfigurationSourceValue(
+          $source,
+          $item);
+      }
+    }
+
+    $result_list = $this->didReadStorageValueList($result_list);
+
+    return $result_list;
+  }
+
+  protected function didReadStorageValueList(array $list) {
+    assert_instances_of($list, 'ArcanistConfigurationSourceValue');
+    return mpull($list, 'getValue');
+  }
+
+}

src/config/option/ArcanistSingleSourceConfigOption.php

@@ -1,6 +1,6 @@
 <?php
 
-abstract class ArcanistScalarConfigOption
+abstract class ArcanistSingleSourceConfigOption
   extends ArcanistConfigOption {
 
   public function getValueFromStorageValueList(array $list) {

src/config/option/ArcanistStringConfigOption.php

@@ -1,7 +1,7 @@
 <?php
 
 final class ArcanistStringConfigOption
-  extends ArcanistScalarConfigOption {
+  extends ArcanistSingleSourceConfigOption {
 
   public function getType() {
     return 'string';

src/config/option/ArcanistStringListConfigOption.php

@@ -0,0 +1,20 @@
+<?php
+
+final class ArcanistStringListConfigOption
+  extends ArcanistListConfigOption {
+
+  public function getType() {
+    return 'list<string>';
+  }
+
+  protected function validateListItem($idx, $item) {
+    if (!is_string($item)) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Expected a string (at index "%s"), found "%s".',
+          $idx,
+          phutil_describe_type($item)));
+    }
+  }
+
+}