Improve order of operations in upgrade script

Summary:
  - Run "phd stop" before stopping apache. This is essentially a smoke test for
PHABRICATOR_ENV being set.
  - Run documentation generation after everything else. Between the pull and the
restart we have some minor exposure to APC issues with deleted files and
out-of-date module definitions, and this limits that.
  - Pull commands out of (x && y) stuff, this prevents "set -e" from working
correctly.

Test Plan: Ran upgrade script locally.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

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

Author: epriestley

scripts/install/update_phabricator.sh

@@ -12,49 +12,42 @@ set -x
 
 ROOT=`pwd` # You can hard-code the path here instead.
 
-
 ### UPDATE WORKING COPIES ######################################################
 
 if [ -e $ROOT/diviner ]
 then
-  (cd $ROOT/diviner && git pull)
+  cd $ROOT/diviner
+  git pull
 fi
 
-(cd $ROOT/libphutil && git pull)
-(cd $ROOT/arcanist && git pull)
-(cd $ROOT/phabricator && git pull && git submodule update --init)
-
+cd $ROOT/libphutil
+git pull
 
-### RUN TESTS ##################################################################
+cd $ROOT/arcanist
+git pull
 
-# This is an acceptance test that makes sure all symboles can be loaded to
-# avoid issues like missing methods in descendants of abstract base class.
-(cd $ROOT/phabricator && ../arcanist/bin/arc unit src/infrastructure/__tests__/)
+cd $ROOT/phabricator
+git pull
+git submodule update --init
 
 
-### GENERATE DOCUMENTATION #####################################################
+### RUN TESTS ##################################################################
 
-# This generates documentation if you have diviner/ checked out. You generally
-# don't need to do this unless you're contributing to Phabricator and want to
-# preview some of the amazing documentation you've just written.
-if [ -e $ROOT/diviner ]
-then
-  (cd $ROOT/diviner && $ROOT/diviner/bin/diviner .)
-  (cd $ROOT/libphutil && $ROOT/diviner/bin/diviner .)
-  (cd $ROOT/arcanist && $ROOT/diviner/bin/diviner .)
-  (cd $ROOT/phabricator && $ROOT/diviner/bin/diviner .)
-fi
+# This is an acceptance test that makes sure all symbols can be loaded to
+# avoid issues like missing methods in descendants of abstract base classes.
+cd $ROOT/phabricator
+../arcanist/bin/arc unit src/infrastructure/__tests__/
 
 
 ### CYCLE APACHE AND DAEMONS ###################################################
 
+# Stop daemons.
+$ROOT/phabricator/bin/phd stop
+
 # Stop Apache. Depening on what system you're running, you may need to use
 # 'apachectl' or something else to cycle apache.
 sudo /etc/init.d/httpd stop
 
-# Stop daemons.
-$ROOT/phabricator/bin/phd stop
-
 # Upgrade the database schema.
 $ROOT/phabricator/scripts/sql/upgrade_schema.php -f
 
@@ -69,3 +62,17 @@ sudo /etc/init.d/httpd start
 # $ROOT/phabricator/bin/phd launch garbagecollector
 # $ROOT/phabricator/bin/phd launch 4 taskmaster
 # $ROOT/phabricator/bin/phd launch ircbot /config/bot.json
+
+
+### GENERATE DOCUMENTATION #####################################################
+
+# This generates documentation if you have diviner/ checked out. You generally
+# don't need to do this unless you're contributing to Phabricator and want to
+# preview some of the amazing documentation you've just written.
+if [ -e $ROOT/diviner ]
+then
+  cd $ROOT/diviner && $ROOT/diviner/bin/diviner .
+  cd $ROOT/libphutil && $ROOT/diviner/bin/diviner .
+  cd $ROOT/arcanist && $ROOT/diviner/bin/diviner .
+  cd $ROOT/phabricator && $ROOT/diviner/bin/diviner .
+fi