Refactor upgrade script, create Shell.eval()

Signed-off-by: Edward Z. Yang <ezyang@mit.edu>

diff --git a/wizard/command/upgrade.py b/wizard/command/upgrade.py
index 7bf73f5b13b99b3047a1dd559213886eb7e07f20..981273cc94ccd1973457580f7f5ea755f15b3406 100644 (file)
--- a/wizard/command/upgrade.py
+++ b/wizard/command/upgrade.py
@@ -9,83 +9,35 @@ import itertools
 
 from wizard import command, deploy, shell, util
 
-# XXX: WARNING EXPERIMENTAL DANGER DANGER WILL ROBINSON
-
-# need errors for checking DAG integrity (if the user is on a completely
-# different history tree, stuff is problems)
+# XXX: need errors for history sanity checking (if the user is on a completely
+# different history tree, we should abort, and manually figure out the
+# appropriate rebase)
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)
-    dir = args[0]
-    command.chdir(dir)
-    if not os.path.isdir(".git"):
-        raise NotAutoinstallError()
-    try:
-        d = deploy.Deployment(".")
-    except IOError as e:
-        if e.errno == errno.ENOENT:
-            raise NotAutoinstallError()
-        else: raise e
+    command.chdir(args[0])
+    d = make_deployment_from_cwd()
     repo = d.application.repository(options.srv_path)
-    # begin the command line process
-    sh = shell.Shell()
-    # setup environment
     util.set_git_env()
-    # commit their changes
+    sh = shell.Shell()
     if not options.dry_run:
-        pre_upgrade_commit(sh)
-    # perform fetch to update repository state
-    sh.call("git", "fetch", repo)
-    # clone their website to a temporary directory
-    temp_dir = tempfile.mkdtemp(prefix="wizard")
-    temp_wc_dir = os.path.join(temp_dir, "repo")
-    logging.info("Using temporary directory: " + temp_wc_dir)
-    sh.call("git", "clone", "--shared", ".", temp_wc_dir)
+        perform_pre_commit(sh)
+    temp_wc_dir = perform_tmp_clone(sh)
     with util.ChangeDirectory(temp_wc_dir):
         if options.dry_run:
-            pre_upgrade_commit(sh)
-        # reconfigure the repository path
+            # we delayed performing the pre upgrade commit
+            # until we're in the temporary directory
+            perform_pre_commit(sh)
         sh.call("git", "remote", "add", "scripts", repo)
         sh.call("git", "fetch", "scripts")
-        # perform the merge
-        version = sh.call("git", "--git-dir="+repo, "describe", "--tags", "master")[0].rstrip()
-        user_commit = sh.call("git", "rev-parse", "HEAD")[0].rstrip()
-        next_commit = sh.call("git", "rev-parse", version)[0].rstrip()
-        message = "Upgraded autoinstall in %s to %s.\n\n%s" % (util.get_dir_owner(), version, util.get_git_footer())
-        try:
-            message += "\nUpgraded-by: " + util.get_operator_git()
-        except util.NoOperatorInfo:
-            pass
-        try:
-            # naive merge algorithm:
-            # sh.call("git", "merge", "-m", message, "scripts/master")
-            # crazy merge algorithm:
-            def make_virtual_commit(tag, parents = []):
-                """WARNING: Changes state of Git repository"""
-                sh.call("git", "checkout", tag, "--")
-                d.parametrize(temp_wc_dir)
-                for file in d.application.parametrized_files:
-                    try:
-                        sh.call("git", "add", "--", file)
-                    except shell.CallError:
-                        pass
-                virtual_tree = sh.call("git", "write-tree")[0].rstrip()
-                parent_args = itertools.chain(*(["-p", p] for p in parents))
-                virtual_commit = sh.call("git", "commit-tree", virtual_tree, *parent_args, input="")[0].rstrip()
-                sh.call("git", "reset", "--hard")
-                return virtual_commit
-            user_tree = sh.call("git", "rev-parse", "HEAD^{tree}")[0].rstrip()
-            base_virtual_commit = make_virtual_commit(d.app_version.pristine_tag)
-            next_virtual_commit = make_virtual_commit(version, [base_virtual_commit])
-            user_virtual_commit = sh.call("git", "commit-tree", user_tree, "-p", base_virtual_commit, input="")[0].rstrip()
-            sh.call("git", "checkout", user_virtual_commit, "--")
-            sh.call("git", "merge", next_virtual_commit) # XXX
-        except shell.CallError:
-            print temp_wc_dir
-            raise MergeFailed
+        version = sh.eval("git", "--git-dir="+repo, "describe", "--tags", "master")
+        user_commit = sh.eval("git", "rev-parse", "HEAD")
+        next_commit = sh.eval("git", "rev-parse", version)
+        perform_merge(sh, repo, d, version)
         # Make it possible to resume here
-        new_tree = sh.call("git", "rev-parse", "HEAD^{tree}")[0].rstrip()
-        final_commit = sh.call("git", "commit-tree", new_tree, "-p", user_commit, "-p", next_commit, input=message)[0].rstrip()
+        new_tree = sh.eval("git", "rev-parse", "HEAD^{tree}")
+        message = make_commit_message(version)
+        final_commit = sh.eval("git", "commit-tree", new_tree, "-p", user_commit, "-p", next_commit, input=message)
         try:
             sh.call("git", "checkout", "-b", "master", "--")
         except shell.CallError:
@@ -97,7 +49,7 @@ def main(argv, baton):
         return
     # XXX: frob .htaccess to make site inaccessible
     # git merge (which performs a fast forward)
-    #   - merge could fail (race)
+    #   - merge could fail (race); that's /really/ dangerous.
     sh.call("git", "pull", temp_wc_dir, "master")
     # run update script
     sh.call(".scripts/update")
@@ -106,7 +58,25 @@ def main(argv, baton):
     #       process might have frobbed it.  Don't be
     #       particularly worried if the segment dissappeared
 
-def pre_upgrade_commit(sh):
+def make_deployment_from_cwd():
+    if not os.path.isdir(".git"):
+        raise NotAutoinstallError()
+    try:
+        d = deploy.Deployment(".")
+    except IOError as e:
+        if e.errno == errno.ENOENT:
+            raise NotAutoinstallError()
+        else: raise e
+    return d
+
+def make_commit_message(version):
+    message = "Upgraded autoinstall in %s to %s.\n\n%s" % (util.get_dir_owner(), version, util.get_git_footer())
+    try:
+        message += "\nUpgraded-by: " + util.get_operator_git()
+    except util.NoOperatorInfo:
+        pass
+
+def perform_pre_commit(sh):
     try:
         message = "Pre-commit of %s locker before autoinstall upgrade.\n\n%s" % (util.get_dir_owner(), util.get_git_footer())
         try:
@@ -118,6 +88,42 @@ def pre_upgrade_commit(sh):
         logging.info("No changes detected")
         pass
 
+def perform_tmp_clone(sh):
+    temp_dir = tempfile.mkdtemp(prefix="wizard")
+    temp_wc_dir = os.path.join(temp_dir, "repo")
+    logging.info("Using temporary directory: " + temp_wc_dir)
+    sh.call("git", "clone", "--shared", ".", temp_wc_dir)
+    return temp_wc_dir
+
+def perform_merge(sh, repo, d, version):
+    try:
+        # naive merge algorithm:
+        # sh.call("git", "merge", "-m", message, "scripts/master")
+        # crazy merge algorithm:
+        def make_virtual_commit(tag, parents = []):
+            """WARNING: Changes state of Git repository"""
+            sh.call("git", "checkout", tag, "--")
+            d.parametrize(".")
+            for file in d.application.parametrized_files:
+                try:
+                    sh.call("git", "add", "--", file)
+                except shell.CallError:
+                    pass
+            virtual_tree = sh.eval("git", "write-tree")
+            parent_args = itertools.chain(*(["-p", p] for p in parents))
+            virtual_commit = sh.eval("git", "commit-tree", virtual_tree, *parent_args, input="")
+            sh.call("git", "reset", "--hard")
+            return virtual_commit
+        user_tree = sh.eval("git", "rev-parse", "HEAD^{tree}")
+        base_virtual_commit = make_virtual_commit(d.app_version.pristine_tag)
+        next_virtual_commit = make_virtual_commit(version, [base_virtual_commit])
+        user_virtual_commit = sh.eval("git", "commit-tree", user_tree, "-p", base_virtual_commit, input="")
+        sh.call("git", "checkout", user_virtual_commit, "--")
+        sh.call("git", "merge", next_virtual_commit)
+    except shell.CallError:
+        print os.getcwd()
+        raise MergeFailed
+
 def parse_args(argv, baton):
     usage = """usage: %prog upgrade [ARGS] DIR
 

diff --git a/wizard/git.py b/wizard/git.py
index aaea68ddb070a7bf15f2d086ad7dca0a66e1d9c9..858ed6b636ee14725ce1830504e6d8b35ca09cc3 100644 (file)
--- a/wizard/git.py
+++ b/wizard/git.py
@@ -6,5 +6,5 @@ from wizard import shell
 
 def describe():
     """Finds the output of git describe --tags of the current directory."""
-    return shell.Shell().safeCall("git", "describe", "--tags")[0].rstrip()
+    return shell.Shell().safeCall("git", "describe", "--tags", strip=True)
 

diff --git a/wizard/install.py b/wizard/install.py
index c9e2f5bc1df6b010a3dd0becf86614defb9d4f0b..f237b8eff4333e1f4af75148fe023f466a18c3b3 100644 (file)
--- a/wizard/install.py
+++ b/wizard/install.py
@@ -64,7 +64,7 @@ import urllib
 import subprocess
 
 import wizard
-from wizard import util
+from wizard import shell, util
 
 def fetch(options, path, post=None):
     """
@@ -140,16 +140,17 @@ class ScriptsMysqlStrategy(Strategy):
     side_effects = True
     def execute(self, options):
         """Attempts to create a database using Scripts utilities."""
+        sh = shell.Shell()
         try:
-            triplet = subprocess.Popen("/mit/scripts/sql/bin/get-password", stdout=subprocess.PIPE).communicate()[0].rstrip().split()
+            triplet = sh.eval("/mit/scripts/sql/bin/get-password").split()
         except:
             raise StrategyFailed
         name = os.path.basename(os.getcwd())
         username = os.getenv('USER')
         options.mysql_host, options.mysql_user, options.mysql_password = triplet
         # race condition
-        options.mysql_db = username + '+' + subprocess.Popen(["/mit/scripts/sql/bin/get-next-database", name], stdout=subprocess.PIPE).communicate()[0].rstrip()
-        subprocess.Popen(["/mit/scripts/sql/bin/create-database", options.mysql_db], stdout=subprocess.PIPE).communicate()
+        options.mysql_db = username + '+' + sh.eval("/mit/scripts/sql/bin/get-next-database", name)
+        sh.call("/mit/scripts/sql/bin/create-database", options.mysql_db)
 
 class ScriptsEmailStrategy(Strategy):
     """Performs script specific guess for email."""

diff --git a/wizard/shell.py b/wizard/shell.py
index 963f3804b4817ec2cecedceda3317bae7b201299..686a782c5a0b2b7b18b03294fc4e4e2644fba4ce 100644 (file)
--- a/wizard/shell.py
+++ b/wizard/shell.py
@@ -44,18 +44,19 @@ class Shell(object):
         :param input: input to feed the subprocess on standard input.
         :param interactive: whether or not directly hook up all pipes
             to the controlling terminal, to allow interaction with subprocess.
-        :returns: a tuple of strings ``(stdout, stderr)``
+        :param strip: if ``True``, instead of returning a tuple,
+            return the string stdout output of the command with trailing newlines
+            removed.  This emulates the behavior of backticks and ``$()`` in Bash.
+            Prefer to use :meth:`eval` instead (you should only need to explicitly
+            specify this if you are using another wrapper around this function).
+        :returns: a tuple of strings ``(stdout, stderr)``, or a string ``stdout``
+            if ``strip`` is specified.
 
         >>> sh = Shell()
         >>> sh.call("echo", "Foobar")
         ('Foobar\\n', '')
-
-        .. note::
-
-            This function does not munge trailing whitespace.  A common
-            idiom for dealing with this is::
-
-                sh.call("echo", "Foobar")[0].rstrip()
+        >>> sh.call("cat", input='Foobar')
+        ('Foobar', '')
         """
         kwargs.setdefault("python", None)
         logging.info("Running `" + ' '.join(args) + "`")
@@ -68,6 +69,7 @@ class Shell(object):
             args[0] = wizard_bin
         kwargs.setdefault("input", None)
         kwargs.setdefault("interactive", False)
+        kwargs.setdefault("strip", False)
         if kwargs["interactive"]:
             stdout=sys.stdout
             stdin=sys.stdin
@@ -96,6 +98,8 @@ class Shell(object):
             if kwargs["python"]: eclass = PythonCallError
             else: eclass = CallError
             raise eclass(proc.returncode, args, stdout, stderr)
+        if kwargs["strip"]:
+            return stdout.rstrip("\n")
         return (stdout, stderr)
     def _log(self, stdout, stderr):
         """Logs the standard output and standard input from a command."""
@@ -144,6 +148,18 @@ class Shell(object):
             return self.callAsUser(*args, **kwargs)
         else:
             return self.call(*args, **kwargs)
+    def eval(self, *args, **kwargs):
+        """
+        Evaluates a command and returns its output, with trailing newlines
+        stripped (like backticks in Bash).  This is a convenience method for
+        calling :meth:`call` with ``strip``.
+
+            >>> sh = Shell()
+            >>> sh.eval("echo", "Foobar") 
+            'Foobar'
+        """
+        kwargs["strip"] = True
+        return self.call(*args, **kwargs)
 
 class ParallelShell(Shell):
     """

diff --git a/wizard/util.py b/wizard/util.py
index 690bc930c4e58c6736aa2d23901d2503709767fb..933be8ab19b17aee9536d5036c4ec5204a075f27 100644 (file)
--- a/wizard/util.py
+++ b/wizard/util.py
@@ -96,6 +96,9 @@ def get_dir_owner(dir = "."):
 
 def get_revision():
     """Returns the commit ID of the current Wizard install."""
+    # If you decide to convert this to use wizard.shell, be warned
+    # that there is a circular dependency, so this function would
+    # probably have to live somewhere else, probably wizard.git
     wizard_git = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), ".git")
     return subprocess.Popen(["git", "--git-dir=" + wizard_git, "rev-parse", "HEAD"], stdout=subprocess.PIPE).communicate()[0].rstrip()