From f08908ff352fe327737b3f055e718e8eb2cc9d76 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 30 Oct 2013 13:07:09 -0700
Subject: [PATCH] Raise a setup warning for missing or invalid local repository
 directory

Summary: I'm planning to add more detailed info to Diffusion itself, but catch the big issue here.

Test Plan: Hit config issue locally, then resolved it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7439
---
 src/__phutil_library_map__.php                |  2 +
 .../PhabricatorSetupCheckRepositories.php     | 43 +++++++++++++++++++
 .../PhabricatorRepositoryConfigOptions.php    | 13 ++----
 3 files changed, 49 insertions(+), 9 deletions(-)
 create mode 100644 src/applications/config/check/PhabricatorSetupCheckRepositories.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 0874f288b2..47a9ccfdb8 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1740,6 +1740,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckPHPConfig' => 'applications/config/check/PhabricatorSetupCheckPHPConfig.php',
     'PhabricatorSetupCheckPath' => 'applications/config/check/PhabricatorSetupCheckPath.php',
     'PhabricatorSetupCheckPygment' => 'applications/config/check/PhabricatorSetupCheckPygment.php',
+    'PhabricatorSetupCheckRepositories' => 'applications/config/check/PhabricatorSetupCheckRepositories.php',
     'PhabricatorSetupCheckStorage' => 'applications/config/check/PhabricatorSetupCheckStorage.php',
     'PhabricatorSetupCheckTimezone' => 'applications/config/check/PhabricatorSetupCheckTimezone.php',
     'PhabricatorSetupIssue' => 'applications/config/issue/PhabricatorSetupIssue.php',
@@ -4070,6 +4071,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckPHPConfig' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckPath' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckPygment' => 'PhabricatorSetupCheck',
+    'PhabricatorSetupCheckRepositories' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckStorage' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckTimezone' => 'PhabricatorSetupCheck',
     'PhabricatorSetupIssueExample' => 'PhabricatorUIExample',
diff --git a/src/applications/config/check/PhabricatorSetupCheckRepositories.php b/src/applications/config/check/PhabricatorSetupCheckRepositories.php
new file mode 100644
index 0000000000..724b538eb9
--- /dev/null
+++ b/src/applications/config/check/PhabricatorSetupCheckRepositories.php
@@ -0,0 +1,43 @@
+<?php
+
+final class PhabricatorSetupCheckRepositories extends PhabricatorSetupCheck {
+
+  protected function executeChecks() {
+    $repo_path = PhabricatorEnv::getEnvConfig('repository.default-local-path');
+
+    if (!$repo_path) {
+      $summary = pht(
+        "The configuration option '%s' is not set.",
+        'repository.default-local-path');
+      $this->newIssue('repository.default-local-path.empty')
+        ->setName(pht('Missing Repository Local Path'))
+        ->setSummary($summary)
+        ->addPhabricatorConfig('repository.default-local-path');
+      return;
+    }
+
+    if (!Filesystem::pathExists($repo_path)) {
+      $summary = pht(
+        "The path for local repositories does not exist, or is not ".
+        "readable by the webserver.");
+      $message = pht(
+        "The directory for local repositories (%s) does not exist, or is not ".
+        "readable by the webserver. Phabricator uses this directory to store ".
+        "information about repositories. If this directory does not exist, ".
+        "create it:\n\n".
+        "%s\n".
+        "If this directory exists, make it readable to the webserver. You ".
+        "can also edit the configuration below to use some other directory.",
+        phutil_tag('tt', array(), $repo_path),
+        phutil_tag('pre', array(), csprintf('$ mkdir -p %s', $repo_path)));
+
+      $this->newIssue('repository.default-local-path.empty')
+        ->setName(pht('Missing Repository Local Path'))
+        ->setSummary($summary)
+        ->setMessage($message)
+        ->addPhabricatorConfig('repository.default-local-path');
+    }
+
+  }
+
+}
diff --git a/src/applications/repository/PhabricatorRepositoryConfigOptions.php b/src/applications/repository/PhabricatorRepositoryConfigOptions.php
index 32fb0bbb33..a641647be2 100644
--- a/src/applications/repository/PhabricatorRepositoryConfigOptions.php
+++ b/src/applications/repository/PhabricatorRepositoryConfigOptions.php
@@ -21,15 +21,10 @@ final class PhabricatorRepositoryConfigOptions
           pht("Default location to store local copies of repositories."))
         ->setDescription(
           pht(
-            "The default location in which to store local copies of ".
-            "repositories. Anything stored in this directory will be assumed ".
-            "to be under the control of Phabricator, which means that ".
-            "Phabricator will try to do some maintenance on working copies ".
-            "if there are problems (such as a change to the remote origin ".
-            "url). This maintenance may include completely removing (and ".
-            "recloning) anything in this directory.\n\n".
-            "When set to null, this option is ignored (i.e. Phabricator will ".
-            "not fully control any working copies).")),
+            "The default location in which to store working copies and other ".
+            "data about repositories. Phabricator will control and manage ".
+            "data here, so you should **not** choose an existing directory ".
+            "full of data you care about.")),
     );
   }