Refactor change directory code to be consistent and permissive.

This fixes bug where if you run wizard upgrade as root in a root
only directory, it fails with permission errors when it attempts
to change back into the directory.

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

diff --git a/TODO b/TODO
index b2ce6b9fd12be0596ee2d0bd133576f2b9e944c6..2420ca6b98358c2aafb23ec1ebf1c0ef06b35173 100644 (file)
--- a/TODO
+++ b/TODO
@@ -4,8 +4,6 @@ TODO NOW:
 
 - Add support for mypristine workflow
 - Make a nicer backtrace if not in a Git working copy directory
 
 - Add support for mypristine workflow
 - Make a nicer backtrace if not in a Git working copy directory

-- Fix chdir to old directory errors
-- Symlinked rerere to get awesomeness. Consider permissions

 - Wordpress needs to get rid of the siteurl hack, so that it actually
   has a fully-qualified URL http://foo.scripts.mit.edu/blah.  This will
   also fix Wordpress's cron functionality.  We should be careful not
 - Wordpress needs to get rid of the siteurl hack, so that it actually
   has a fully-qualified URL http://foo.scripts.mit.edu/blah.  This will
   also fix Wordpress's cron functionality.  We should be careful not


@@ -22,6 +20,7 @@ TODO NOW:
      http://peak.telecommunity.com/DevCenter/PkgResources#entry-points
      https://xvm.mit.edu:1111/trunk/packages/invirt-base/python/invirt/authz.py
      https://xvm.mit.edu:1111/trunk/packages/xvm-authz-locker/setup.py
      http://peak.telecommunity.com/DevCenter/PkgResources#entry-points
      https://xvm.mit.edu:1111/trunk/packages/invirt-base/python/invirt/authz.py
      https://xvm.mit.edu:1111/trunk/packages/xvm-authz-locker/setup.py

+     http://pylonshq.com/docs/en/0.9.7/advanced_pylons/entry_points_and_plugins/

 
 - Remerges aren't reflected in the parent files, so `git diff` output is
   spurious.  Not sure how to fix this w/o tree hackery.
 
 - Remerges aren't reflected in the parent files, so `git diff` output is
   spurious.  Not sure how to fix this w/o tree hackery.


@@ -50,13 +49,6 @@ TODO NOW:
       namely cooking up the sudo and environment variable lines
     - Summary script should be more machine friendly, and should not
       output summary charts when I increase specificity
       namely cooking up the sudo and environment variable lines
     - Summary script should be more machine friendly, and should not
       output summary charts when I increase specificity

-    - Report code in wizard/command/__init__.py is ugly as sin.  Also,
-      the Report object should operate at a higher level of abstraction
-      so we don't have to manually increment fails. (in fact, that should
-      probably be called something different).  The by-percent errors should
-      also be automated.
-    - Move resolutions in mediawiki.py to a text file? (the parsing overhead
-      may not be worth it)

     - PHP end of file allows omitted semicolon, can result in parse error
       if merge resolutions aren't careful.  `php -l` can be a quick stopgap
 
     - PHP end of file allows omitted semicolon, can result in parse error
       if merge resolutions aren't careful.  `php -l` can be a quick stopgap
 

diff --git a/wizard/command/backup.py b/wizard/command/backup.py
index d7be811414c33e566d12cdac1a0fa1cbc35e41eb..1961616b8ee5c259a1b0412e4a6361da1a95ce43 100644 (file)
--- a/wizard/command/backup.py
+++ b/wizard/command/backup.py
@@ -1,11 +1,10 @@
+import os.path
+

 from wizard import command, deploy, shell, util
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)
 from wizard import command, deploy, shell, util
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)

-    if not args:
-        dir = "."
-    else:
-        dir = args[0]
+    dir = os.path.abspath(args[0]) if args else os.getcwd()

     shell.drop_priviledges(dir, options.log_file)
     util.chdir(dir)
     d = deploy.ProductionCopy(".")
     shell.drop_priviledges(dir, options.log_file)
     util.chdir(dir)
     d = deploy.ProductionCopy(".")

diff --git a/wizard/command/blacklist.py b/wizard/command/blacklist.py
index 535fdb307b46026c6e64abb356fe873256367f51..f20e43d8b6748460d24368d8d41625d69c8d1089 100644 (file)
--- a/wizard/command/blacklist.py
+++ b/wizard/command/blacklist.py
@@ -8,6 +8,9 @@ def main(argv, baton):
     # XXX: this should be abstracted away!
     if os.path.exists(".git/WIZARD_REPO"):
         util.chdir(shell.eval('git', 'config', 'remote.origin.url'))
     # XXX: this should be abstracted away!
     if os.path.exists(".git/WIZARD_REPO"):
         util.chdir(shell.eval('git', 'config', 'remote.origin.url'))

+    # Directory information not transferred via command line, so this
+    # will not error due to the changed directory.
+    shell.drop_priviledges(".", options.log_file)

     open('.scripts/blacklisted', 'w').write(reason + "\n")
 
 def parse_args(argv, baton):
     open('.scripts/blacklisted', 'w').write(reason + "\n")
 
 def parse_args(argv, baton):

diff --git a/wizard/command/database.py b/wizard/command/database.py
index ee593216445307a022db5488c94538fa5e911538..c2a1d4d6f635b1a472f21702c3be7e7f4a83d1c9 100644 (file)
--- a/wizard/command/database.py
+++ b/wizard/command/database.py
@@ -1,13 +1,16 @@
-from wizard import deploy, command
+import os.path
+
+from wizard import deploy, command, shell

 
 def main(argv, baton):
     options, args = parse_args(argv, baton)
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)

-    dir = args[0]
+    dir = os.path.abspath(args[0]) if args else os.getcwd()
+    shell.drop_priviledges(dir, options.log_file)

     deployment = deploy.ProductionCopy(dir)
     print deployment.dsn.database
 
 def parse_args(argv, baton):
     deployment = deploy.ProductionCopy(dir)
     print deployment.dsn.database
 
 def parse_args(argv, baton):

-    usage = """usage: %prog database DIR
+    usage = """usage: %prog database [DIR]

 
 Prints the name of the database an application is using.
 Maybe in the future this will print more information."""
 
 Prints the name of the database an application is using.
 Maybe in the future this will print more information."""

diff --git a/wizard/command/migrate.py b/wizard/command/migrate.py
index 259d2e747861ae5728f061d3b2ff9923819b88ec..829261f0866c8d7a87fde8f7f0b4ae8887c45eaf 100644 (file)
--- a/wizard/command/migrate.py
+++ b/wizard/command/migrate.py
@@ -1,4 +1,5 @@
 import os
 import os

+import os.path

 import shutil
 import logging
 
 import shutil
 import logging
 


@@ -6,14 +7,10 @@ from wizard import command, deploy, shell, util
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)

-    if args:
-        dir = args[0]
-    else:
-        dir = os.getcwd()
-
+    dir = os.path.abspath(args[0]) if args else os.getcwd()

     shell.drop_priviledges(dir, options.log_file)
     shell.drop_priviledges(dir, options.log_file)

-

     util.chdir(dir)
     util.chdir(dir)

+

     sh = shell.Shell(options.dry_run)
 
     logging.info("Migrating %s" % dir)
     sh = shell.Shell(options.dry_run)
 
     logging.info("Migrating %s" % dir)

diff --git a/wizard/command/quota.py b/wizard/command/quota.py
index 2a640c91f26db96458a434c15dce81b1418f37d5..18ae8d014c592fcc72ec7a25703487a6c9f9bd27 100644 (file)
--- a/wizard/command/quota.py
+++ b/wizard/command/quota.py
@@ -5,11 +5,8 @@ import sys
 from wizard import command, scripts
 
 def main(argv, baton):
 from wizard import command, scripts
 
 def main(argv, baton):

-    options, dirs = parse_args(argv, baton)
-    if not dirs:
-        dir = os.getcwd()
-    else:
-        dir = dirs[0]
+    options, args = parse_args(argv, baton)
+    dir = os.path.abspath(args[0]) if args else os.getcwd()

     r = scripts.get_quota_usage_and_limit(dir)
     if r[0] is None or r[1] is None:
         sys.exit(1)
     r = scripts.get_quota_usage_and_limit(dir)
     if r[0] is None or r[1] is None:
         sys.exit(1)

diff --git a/wizard/command/upgrade.py b/wizard/command/upgrade.py
index 9b1c152243ddab31d0d063969ba7410e5eb4063f..e56675bbe6aaf8e74d7452df1c692b9229efc06a 100644 (file)
--- a/wizard/command/upgrade.py
+++ b/wizard/command/upgrade.py
@@ -1,6 +1,7 @@
 import sys
 import distutils.version
 import os
 import sys
 import distutils.version
 import os

+import os.path

 import shutil
 import logging.handlers
 import tempfile
 import shutil
 import logging.handlers
 import tempfile


@@ -15,10 +16,8 @@ errno_blacklisted = 64
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)
 
 def main(argv, baton):
     options, args = parse_args(argv, baton)

-    if args:
-        dir = args[0]
-    else:
-        dir = "."
+    dir = os.path.abspath(args[0]) if args else os.getcwd()
+    os.chdir(dir)

     shell.drop_priviledges(dir, options.log_file)
     util.set_git_env()
     upgrade = Upgrade(options)
     shell.drop_priviledges(dir, options.log_file)
     util.set_git_env()
     upgrade = Upgrade(options)


@@ -75,27 +74,28 @@ class Upgrade(object):
 
     def execute(self, dir):
         """
 
     def execute(self, dir):
         """

-        Executes an upgrade.  This is the entry-point.
+        Executes an upgrade.  This is the entry-point.  This expects
+        that it's current working directory is the same as ``dir``.

         """
         """

-        with util.ChangeDirectory(dir):
-            try:
-                if self.options.continue_:
-                    logging.info("Continuing upgrade...")
-                    self.resume()
-                else:
-                    logging.info("Upgrading %s" % os.getcwd())
-                    self.preflight()
-                    self.merge()
-                self.postflight()
-                # Till now, all of our operations were in a tmp sandbox.
-                if self.options.dry_run:
-                    logging.info("Dry run, bailing.  See results at %s" % self.temp_wc_dir)
-                    return
-                backup = self.backup()
-                self.upgrade(backup)
-            finally:
-                if self.use_shm and self.temp_dir and os.path.exists(self.temp_dir):
-                    shutil.rmtree(self.temp_dir)
+        assert os.path.abspath(dir) == os.getcwd()
+        try:
+            if self.options.continue_:
+                logging.info("Continuing upgrade...")
+                self.resume()
+            else:
+                logging.info("Upgrading %s" % os.getcwd())
+                self.preflight()
+                self.merge()
+            self.postflight()
+            # Till now, all of our operations were in a tmp sandbox.
+            if self.options.dry_run:
+                logging.info("Dry run, bailing.  See results at %s" % self.temp_wc_dir)
+                return
+            backup = self.backup()
+            self.upgrade(backup)
+        finally:
+            if self.use_shm and self.temp_dir and os.path.exists(self.temp_dir):
+                shutil.rmtree(self.temp_dir)

 
     def resume(self):
         """
 
     def resume(self):
         """