From: Edward Z. Yang Date: Tue, 2 Mar 2010 02:06:54 +0000 (-0500) Subject: Refactor change directory code to be consistent and permissive. X-Git-Url: https://scripts.mit.edu/gitweb/wizard.git/commitdiff_plain/ad795d92aed9f47636c46a2a4604253be06eb526 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 --- diff --git a/TODO b/TODO index b2ce6b9..2420ca6 100644 --- 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 -- 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 @@ -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://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. @@ -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 - - 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 diff --git a/wizard/command/backup.py b/wizard/command/backup.py index d7be811..1961616 100644 --- 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) - 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(".") diff --git a/wizard/command/blacklist.py b/wizard/command/blacklist.py index 535fdb3..f20e43d 100644 --- 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')) + # 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): diff --git a/wizard/command/database.py b/wizard/command/database.py index ee59321..c2a1d4d 100644 --- 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) - 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): - 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.""" diff --git a/wizard/command/migrate.py b/wizard/command/migrate.py index 259d2e7..829261f 100644 --- a/wizard/command/migrate.py +++ b/wizard/command/migrate.py @@ -1,4 +1,5 @@ import os +import os.path 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) - 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) - util.chdir(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 2a640c9..18ae8d0 100644 --- 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): - 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) diff --git a/wizard/command/upgrade.py b/wizard/command/upgrade.py index 9b1c152..e56675b 100644 --- a/wizard/command/upgrade.py +++ b/wizard/command/upgrade.py @@ -1,6 +1,7 @@ import sys import distutils.version import os +import os.path import shutil import logging.handlers import tempfile @@ -15,10 +16,8 @@ errno_blacklisted = 64 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) @@ -75,27 +74,28 @@ class Upgrade(object): 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): """