Fix bugs, add better logging.

* dry_run semantics were improperly being triggered
  due to a logic bug.
* Created warnings.log and errors.log files, which
  are machine friendly lists of warning'd and error'd
  installs.
* Add the shorten number to make it easy for someone
  to grab a specific logfile.
* Add locking to migration; this means better error
  messages too
* Make call errors more expressive.

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

diff --git a/TODO b/TODO
index 2f4e64613f3e76133ff37744d29fe53ddb261d59..e4d5cb61659a213e7111688417d207bfbc657a02 100644 (file)
--- a/TODO
+++ b/TODO
@@ -2,14 +2,11 @@ The Git Autoinstaller
 
 TODO NOW:
 
-- Genericize callAsUser and drop_priviledges in shell
 - Remove "already migrated" cruft that will accumulate if we do small
   --limit and then increase.
-- Make sure to generate reports about what errored and what had warnings.
-  Same goes for our output
-- Allow to migrate just one user (user filtering of installs)
-- Make sure MediaWiki upgrade script does not give proper exit code
-  if it fails, so be sure to check for "Done" in the last 10 characters.
+- Allow to migrate just one user (user filtering of installs, also
+  has userland capabilities, although it means we need some way of
+  selectively publishing the versions directory)
 
 - Make parallel-find.pl use `sudo -u username git describe --tags`
   to determine version.  Make parallel-find.pl have this have greater
@@ -22,6 +19,8 @@ TODO NOW:
 - Better error message if daemon/scripts-security-upd
   is not on scripts-security-upd list
 
+- MediaWiki upgrade script does not give proper exit code;
+  if it fails, so be sure to check for "Done" in the last 10 characters.
 - Custom merge algo: absolute php.ini symlinks to relative symlinks
 - Custom merge algo: re-constitute AdminSettings.php if missing.  It looks
   like this is the case for most 1.5.8 installs (check what the merges
@@ -33,6 +32,7 @@ TODO NOW:
 - Redo Wordpress conversion, with an eye for automating everything
   possible (such as downloading the tarball and unpacking)
 
+- Genericize callAsUser and drop_priviledges in shell
 - Summary script should be more machine friendly, and should not
   output summary charts when I increase specificity
 

diff --git a/wizard/command/__init__.py b/wizard/command/__init__.py
index 3e0f43fc2627851edb013a946d45ba4101fb9049..505142009796322c387d3f4640b9f6e65b335ef5 100644 (file)
--- a/wizard/command/__init__.py
+++ b/wizard/command/__init__.py
@@ -83,7 +83,7 @@ def makeLogger(options, numeric_args):
         logger.setLevel(logging.DEBUG)
     else:
         stderr.setLevel(logging.WARNING)
-        if options.verbose or hasattr(options, "dry_run"):
+        if options.verbose or hasattr(options, "dry_run") and options.dry_run:
             stderr.setLevel(logging.INFO)
         if options.log_file:
             file.setLevel(logging.INFO)

diff --git a/wizard/command/mass_migrate.py b/wizard/command/mass_migrate.py
index 670b6a064fcef802949410a30661bb654c0f765d..88f008736d26599a27fc2b90326a61b95f9471df 100644 (file)
--- a/wizard/command/mass_migrate.py
+++ b/wizard/command/mass_migrate.py
@@ -18,6 +18,8 @@ def main(argv, baton):
     sh = make_shell(options)
     seen = make_serialized_set(options)
     my_uid = os.getuid() # to see if we have root
+    warnings_logname = "/tmp/wizard-migrate-warnings.log"
+    errors_logname = "/tmp/wizard-migrate-errors.log"
     if options.log_dir:
         # must not be on AFS
         try:
@@ -29,6 +31,10 @@ def main(argv, baton):
                 options.log_dir = os.path.join(options.log_dir, str(int(time.time())))
                 os.mkdir(options.log_dir) # if fails, be fatal
         os.chmod(options.log_dir, 0o777)
+        warnings_logname = os.path.join(options.log_dir, "warnings.log")
+        errors_logname = os.path.join(options.log_dir, "errors.log")
+    warnings_log = open(warnings_logname, "a")
+    errors_log = open(errors_logname, "a")
     # loop stuff
     errors = {}
     i = 0
@@ -60,8 +66,11 @@ def main(argv, baton):
             log_file = os.path.join(options.log_dir, shorten(i, d.location))
             child_args.append("--log-file=" + log_file)
         # actual meat
-        def make_on_pair(d):
+        def make_on_pair(d, i):
             def on_success(stdout, stderr):
+                if stderr:
+                    warnings_log.write("%s\n" % d.location)
+                    logging.warning("Warnings [%04d] %s:\n%s" % (d.location, i, stderr))
                 seen.add(d.location)
             def on_error(e):
                 if e.name == "wizard.command.migrate.AlreadyMigratedError" or \
@@ -72,9 +81,10 @@ def main(argv, baton):
                     name = e.name
                     if name not in errors: errors[name] = []
                     errors[name].append(d)
-                    logging.error("%s in %s" % (name, d.location))
+                    logging.error("%s in [%04d] %s" % (name, i, d.location))
+                    errors_log.write("%s\n" % d.location)
             return (on_success, on_error)
-        on_success, on_error = make_on_pair(d)
+        on_success, on_error = make_on_pair(d, i)
         sh.wait() # wait for a parallel processing slot to be available
         sh.call("wizard", "migrate", d.location, *child_args,
                       on_success=on_success, on_error=on_error)
@@ -125,7 +135,6 @@ untrusted repositories."""
 
 def calculate_base_args(options):
     base_args = command.makeBaseArgs(options, dry_run="--dry-run", srv_path="--srv-path", force="--force")
-    base_args += '--quiet'
     return base_args
 
 def shorten(i, dir):

diff --git a/wizard/command/migrate.py b/wizard/command/migrate.py
index 2570dce55086c117b7ae91868b9924cfb141948c..56b4e61dda6be6fa64f8aa5d76458ecb960ce71e 100644 (file)
--- a/wizard/command/migrate.py
+++ b/wizard/command/migrate.py
@@ -28,8 +28,21 @@ def main(argv, baton):
 
     os.unsetenv("GIT_DIR") # prevent some perverse errors
 
-    make_repository(sh, options, repo, tag)
-    check_variables(deployment, options)
+    try:
+        try:
+            os.fdopen(os.open(".scripts-migrate-lock", os.O_CREAT | os.O_EXCL)).write(str(os.getpid()))
+        except OSError as e:
+            if e.errno == errno.EEXIST:
+                raise DirectoryLockedError
+            elif e.errno == errno.EACCES:
+                raise command.PermissionsError(dir)
+        make_repository(sh, options, repo, tag)
+        check_variables(deployment, options)
+    finally:
+        try:
+            os.unlink(".scripts-migrate-lock")
+        except OSError:
+            pass
 
 def parse_args(argv, baton):
     usage = """usage: %prog migrate [ARGS] DIR
@@ -201,6 +214,16 @@ ERROR: Could not find .scripts-version file. Are you sure
 this is an autoinstalled application?
 """
 
+class DirectoryLockedError(Error):
+    def __init__(self, dir):
+        self.dir = dir
+    def __str__(self):
+        return """
+
+ERROR: Could not acquire lock on directory.  Maybe there is
+another migration process running?
+"""
+
 class NoTagError(Error):
     def __init__(self, version):
         self.version = version

diff --git a/wizard/shell.py b/wizard/shell.py
index fcc93774968f8d71fff7868b561391f07f0d2f12..36f23331d7366349244971d7ef18afdfe9284108 100644 (file)
--- a/wizard/shell.py
+++ b/wizard/shell.py
@@ -325,7 +325,7 @@ class CallError(Error):
         self.stdout = stdout
         self.stderr = stderr
     def __str__(self):
-        return "CallError [%d]" % self.code
+        return "CallError [%d]\n%s" % (self.code, self.stderr)
 
 class PythonCallError(CallError):
     """
@@ -339,7 +339,7 @@ class PythonCallError(CallError):
         CallError.__init__(self, code, args, stdout, stderr)
     def __str__(self):
         if self.name:
-            return "PythonCallError [%s]" % self.name
+            return "PythonCallError [%s]\n%s" % (self.name, self.stderr)
         else:
-            return "PythonCallError"
+            return "PythonCallError\n%s" % self.stderr