Code cleanup, fix two bugs

delete

@@ -4,30 +4,14 @@ import logging
 import optparse
 import os
 import shutil
-import stat
 import sys
 
-import afs.fs
+import libdelete
+from libdelete import perror
 
 logger = logging.getLogger('delete')
 whoami = os.path.basename(sys.argv[0])
 
-import libdelete
-
-def debug_callback(option, opt_str, value, parser):
-    """
-    An OptionParser callback that enables debugging.
-    """
-    all_loggers = [logger.name, 'libdelete']
-    loggers = [x.strip() for x in value.split(',')]
-    if value.lower() == 'all':
-        loggers = all_loggers
-    else:
-        if not set(loggers) == set(all_loggers):
-            parser.error('Valid debug targets: {0}'.format(
-                    ", ".join(all_loggers)))
-    for l in loggers:
-        logging.getLogger(l).setLevel(logging.DEBUG)
 
 def ask(question, *args, **kwargs):
     """
@@ -37,21 +21,11 @@ def ask(question, *args, **kwargs):
     yes = ('y', 'yes')
     prepend = '' if kwargs.get('nowhoami', False) else "{0}: ".format(whoami)
     try:
-        return raw_input("%s%s " % (prepend,
-                                    question % args)).strip().lower() in yes
+        return input("%s%s " % (prepend,
+                                question % args)).strip().lower() in yes
     except KeyboardInterrupt:
         sys.exit(0)
 
-def perror(message, **kwargs):
-    """
-    Format an error message, log it in the debug log
-    and maybe also print it to stderr.
-    """
-    should_print = kwargs.pop('_maybe', False)
-    msg = "{0}: {1}".format(whoami, message.format(**kwargs))
-    logger.debug("Error: %s", msg)
-    if should_print:
-        print >>sys.stderr, msg
 
 def actually_delete(filename, options):
     """
@@ -67,7 +41,8 @@ def actually_delete(filename, options):
                    filename):
             return False
     if options.noop:
-        print >>sys.stderr, "{0}: {1} would be removed".format(whoami, filename)
+        sys.stderr.write("{0}: {1} would be removed\n".format(
+            whoami, filename))
         return True
     (dirname, basename) = os.path.split(filename)
     newname = os.path.join(dirname, '.#' + basename)
@@ -82,9 +57,14 @@ def actually_delete(filename, options):
     # Maybe we can just use os.rename here
     shutil.move(filename, newname)
     # None means use the current time
-    os.utime(newname, None)
+    try:
+        os.utime(newname, None)
+    except PermissionError:
+        # User does not own the deleted file, *shrug* we did the best we can
+        pass
     return True
 
+
 def delete(filename, options):
     logger.debug("delete(%s)", filename)
     if not os.path.lexists(filename):
@@ -112,7 +92,8 @@ def delete(filename, options):
                 is_empty = libdelete.empty_directory(filename)
             except OSError as e:
                 # Do we want to only do this if emulating rm?
-                print >>sys.stderr, ": ".join((whoami, e.filename, e.strerror))
+                sys.stderr.write("{}: {}: {}\n".format(
+                    whoami, e.filename, e.strerror))
                 return False
             if is_empty:
                 return actually_delete(filename, options)
@@ -135,6 +116,7 @@ def delete(filename, options):
         else:
             return actually_delete(filename, options)
 
+
 def main():
     parser = optparse.OptionParser(usage="%prog [options] filename ...")
     # This is probably a terrible idea, but the old code did it
@@ -167,8 +149,9 @@ def main():
         default=linked_to_rmdir,
         help="Only remove empty directories (refuse to remove files)")
     parser.add_option(
-        "--debug", action="callback", type='string', help="Enable debugging (logger target or 'all')",
-        callback=debug_callback, metavar='target')
+        "--debug", action="callback", type='string',
+        help="Enable debugging (logger target or 'all')",
+        callback=libdelete.make_debug_callback(logger), metavar='target')
     (options, args) = parser.parse_args()
     if options.filesonly and options.directoriesonly:
         parser.error("-F and -D are mutually exclusive")
@@ -181,13 +164,13 @@ def main():
     for filename in args:
         # Because you know _someone_ will try it
         if len(filename.rstrip('/')) < 1:
-            print >>sys.stderr, "That's not a good idea."
-            sys.exit(1)
+            sys.exit("That's not a good idea.")
         # Trailing slashes make bad things happen
         if not delete(filename.rstrip('/'), options):
             errors = 1
     return errors
 
+
 if __name__ == "__main__":
     logging.basicConfig(level=logging.WARNING)
     sys.exit(main())

expunge

@@ -6,6 +6,7 @@ import os
 import sys
 
 import libdelete
+from libdelete import perror
 
 header = "The following deleted files are going to be expunged:\n"
 footer = """
@@ -17,20 +18,6 @@ confirmation = "Do you wish to continue [return = no]? "
 logger = logging.getLogger('expunge')
 whoami = os.path.basename(sys.argv[0])
 
-def debug_callback(option, opt_str, value, parser):
-    """
-    An OptionParser callback that enables debugging.
-    """
-    all_loggers = [logger.name, 'libdelete']
-    loggers = [x.strip() for x in value.split(',')]
-    if value.lower() == 'all':
-        loggers = all_loggers
-    else:
-        if not set(loggers) == set(all_loggers):
-            parser.error('Valid debug targets: {0}'.format(
-                    ", ".join(all_loggers)))
-    for l in loggers:
-        logging.getLogger(l).setLevel(logging.DEBUG)
 
 def ask(question, *args, **kwargs):
     """
@@ -40,35 +27,27 @@ def ask(question, *args, **kwargs):
     yes = ('y', 'yes')
     prepend = '' if kwargs.get('nowhoami', False) else "{0}: ".format(whoami)
     try:
-        return raw_input("%s%s " % (prepend,
-                                    question % args)).strip().lower() in yes
+        return input("%s%s " % (prepend,
+                                question % args)).strip().lower() in yes
     except KeyboardInterrupt:
         sys.exit(0)
 
-def perror(message, **kwargs):
-    """
-    Format an error message, log it in the debug log
-    and maybe also print it to stderr.
-    """
-    should_print = not kwargs.pop('_maybe', False)
-    msg = "{0}: {1}".format(whoami, message.format(**kwargs))
-    logger.debug("Error: %s", msg)
-    if should_print:
-        print >>sys.stderr, msg
 
 def getsize(path):
     size = os.path.getsize(path)
     return (size, "(%dKB)" % (libdelete.to_kb(size),))
 
+
 def expunge(deleted_files, options):
     expunged_size = 0
     errors = 0
     if options.listfiles:
-        print header
-        print libdelete.format_columns(sorted(
+        print(header)
+        sys.stdout.write(libdelete.format_columns(
+            sorted(
                 [libdelete.relpath(
-                        libdelete.undeleted_name(x)) for x in deleted_files]))
-        print footer
+                    libdelete.undeleted_name(x)) for x in deleted_files])))
+        print(footer)
         if not options.force and \
                 not ask(confirmation, nowhoami=True):
             logger.debug("User failed to confirm; exiting")
@@ -97,7 +76,11 @@ def expunge(deleted_files, options):
                     # We exit here, not keep going, as the original code did
                     sys.exit(errors)
             if options.verbose:
-                print "{whoami}: {path} {size} {maybe}expunged ({total}KB total)".format(whoami=whoami, path=f, size=size_str, maybe='would be ' if options.noop else '', total=libdelete.to_kb(expunged_size))
+                print("{whoami}: {path} {size} {maybe}expunged "
+                      "({total}KB total)".format(
+                          whoami=whoami, path=f, size=size_str,
+                          maybe='would be ' if options.noop else '',
+                          total=libdelete.to_kb(expunged_size)))
         if not options.noop:
             if os.path.isdir(f) and not os.path.islink(f):
                 logger.debug("rmdir: %s", f)
@@ -107,9 +90,10 @@ def expunge(deleted_files, options):
                 os.unlink(f)
 
     if options.yieldsize:
-        print "Total expunged: {0}KB".format(libdelete.to_kb(expunged_size))
+        print("Total expunged: {0}KB".format(libdelete.to_kb(expunged_size)))
     return errors
 
+
 def parse_options():
     parser = optparse.OptionParser(usage="%prog [options] filename ...")
     parser.add_option(
@@ -143,22 +127,23 @@ def parse_options():
         "-m", dest="f_mounts", action="store_true", default=False,
         help="Follow mount points")
     parser.add_option(
-        "--debug", action="callback", type='string', help="Enable debugging (logger target or 'all')",
-        callback=debug_callback, metavar='target')
+        "--debug", action="callback", type='string',
+        help="Enable debugging (logger target or 'all')",
+        callback=libdelete.make_debug_callback(logger), metavar='target')
     (options, args) = parser.parse_args()
     if options.noop:
         # -n implies -v
         options.verbose = True
     return (options, args)
 
+
 def main():
     rv = 0
     if ((whoami == "purge") and len(sys.argv) > 1):
         if (len(sys.argv) == 2) and (sys.argv[1] == '--debug'):
             sys.argv.append('all')
         else:
-            print >>sys.stderr, "purge does not take any arguments or options."
-            sys.exit(1)
+            sys.exit("purge does not take any arguments or options.")
     (options, args) = parse_options()
     if (whoami == "purge"):
         args = [os.path.expanduser('~')]
@@ -185,11 +170,13 @@ def main():
             # Doesn't cover all corner cases, but covers everything the old
             # code supported.  In particular, weird symlinks will make this
             # sad
-            deleted_files.sort(reverse=True, key=lambda x: x.count(os.path.sep))
+            deleted_files.sort(reverse=True,
+                               key=lambda x: x.count(os.path.sep))
             rv += expunge(deleted_files, options)
 
     return rv
 
+
 if __name__ == "__main__":
     logging.basicConfig(level=logging.WARNING)
     sys.exit(main())