Skip to main content
Tweeted twitter.com/#!/StackCodeReview/status/622312614761263108
deleted 125 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

Code can be found on Github.

Some coworkers and I sometimes want to delete folders, only to get the very informative: Permission Denied"Permission Denied" message. I had discovered Handle.exe and Process Explorer to help with this, but the method was still too manual and cumbersome.

Thus, I decided to write this Python CLI script to release locks on a given file. It runs Handle.exeHandle.exe under the hood and parses the output to prompt the user for confirmation to kill any given process (unless a quiet-like option flag is given).

We all work on Windows 7 using basically the same setup (IT takes care of setting up all the machines). The default Python is ActivateState's Python2.6.9. While some people use cmd.execmd.exe, a lot of guys use BashBash or CygwinCygwin, so I wanted to support all of these.

EDIT: As per user request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

Code can be found on Github.

Some coworkers and I sometimes want to delete folders, only to get the very informative: Permission Denied message. I had discovered Handle.exe and Process Explorer to help with this, but the method was still too manual and cumbersome.

Thus, I decided to write this Python CLI script to release locks on a given file. It runs Handle.exe under the hood and parses the output to prompt the user for confirmation to kill any given process (unless a quiet-like option flag is given)

We all work on Windows 7 using basically the same setup (IT takes care of setting up all the machines). The default Python is ActivateState's Python2.6.9. While some people use cmd.exe, a lot of guys use Bash or Cygwin, so I wanted to support all of these.

EDIT: As per user request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

Some coworkers and I sometimes want to delete folders, only to get the very informative: "Permission Denied" message. I had discovered Handle.exe and Process Explorer to help with this, but the method was still too manual and cumbersome.

Thus, I decided to write this Python CLI script to release locks on a given file. It runs Handle.exe under the hood and parses the output to prompt the user for confirmation to kill any given process (unless a quiet-like option flag is given).

We all work on Windows 7 using basically the same setup (IT takes care of setting up all the machines). The default Python is ActivateState's Python2.6.9. While some people use cmd.exe, a lot of guys use Bash or Cygwin, so I wanted to support all of these.

I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

deleted 2 characters in body
Source Link
selimb
  • 173
  • 4

EDIT: As per @janosuser request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

EDIT: As per @janos request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

EDIT: As per user request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

added 6369 characters in body
Source Link
selimb
  • 173
  • 4

EDIT: As per @janos request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

Also, I'm at a loss as to how to test this. I've added tests for abspath function (see repo), but I'm having trouble writing tests for the other functions.

"""Dammit let me delete this file!

Usage:
    dammit [-y | -s] <name>
    dammit (-h)

Options:
-y        Kill without permission.
-s        Enable hotkey
-h        Show this screen.

"""
from __future__ import print_function

from docopt import docopt
from functools import partial
import logging
import msvcrt
import os
import posixpath
import re
import subprocess
import sys
import time

log = logging.getLogger()
log.setLevel(logging.INFO)
ch = logging.StreamHandler()
ch.setLevel(logging.INFO)
PREFIX = 'dammit:'
formatter = logging.Formatter(PREFIX + ' %(message)s')
ch.setFormatter(formatter)
log.addHandler(ch)

CYGPATH = ['C:\\GnuNT\\bin\\cygpath.exe']
HANDLE = ["handle64.exe"]
# handle.exe provides way to close specified handle, which would be preferable,
# but this requires administrator rights :(
KILL = ["TASKKILL", "/f", "/pid"]
ACTIVATE = ["activatePID.exe"]
EXPLORER = "explorer.exe"
CANT_EXECUTE = """Could not execute command: %s"""


def abspath(path):
    """Return the absolute path of given `path`.

    Taken to be relative from current working directory if input is not an
    absolute path.

    Args:
        path (str) : Pathname to absolutize.

    Notes:
        Should work for POSIX and Windows paths to accomodate
        different shells.
    """
    if posixpath.isabs(path):  # Starts with "/"
        if path[1:9] != 'cygdrive':
            path = posixpath.join('/cygdrive', path[1:])
        proc = subprocess.Popen(CYGPATH + ['-w', path],
                                stdout=subprocess.PIPE)
        return proc.stdout.read().rstrip()  # Remove trailing newline
    return os.path.abspath(path)


def find_locks(name):
    """Find which processes currently have a particular file/directory open.

    Args:
        name (str) : Name of the file which is locked.

    Returns:
        List of (process, PID)-pairs which are accessing the file.
    """
    try:
        output = subprocess.check_output(HANDLE + [name])
    except WindowsError:
        log.info(CANT_EXECUTE % HANDLE)
        sys.exit(1)
    except subprocess.CalledProcessError:
        return None
    pattern = r"([^\s]+)(?:\s+)pid: ([0-9]{4})"
    return frozenset(re.findall(pattern, output))


def kill(pid):
    """Kill process with given PID

    Args:
        pid (str) : PID to kill
    """
    try:
        with open(os.devnull, 'w') as nul:
            subprocess.Popen(KILL + [str(pid)], stdout=nul, stderr=nul)
    except WindowsError:
        log.info(CANT_EXECUTE % KILL)
        sys.exit(1)
    return


def _activatePID(pid, enable_hotkeys):
    """Try to activate window associated with given process ID

    Args:
        pid (str) : PID of process to activate.
        enable_hotkeys (bool) : If True, Autohotkey will send "y" and "n"
            key presses to the prompt upon activating requested window.
    """
    try:
        subprocess.Popen(ACTIVATE + [pid, str(enable_hotkeys)])
    except WindowsError:
        log.info(CANT_EXECUTE % ACTIVATE)
    return


def query(process, pid, show_callback):
    """Query user to kill or show process.

    No invalid answers allowed.

    Args:
        process (str) : Process name
        pid (str) : Process PID
        show_callback (func(x)) : Ran when user answers "show"


    Answers:
        y : Kill process
        n : Do not kill process
        s : Try to show (activate) window linked to the process.

    Answering "show" does not exit the prompt.

    Returns:
        bool: True if yes, False if no.
    """
    question = " Kill process %s with PID %s [y/n/s]? " % (process, pid)
    sys.stdout.write(PREFIX + question)
    while True:
        answer = msvcrt.getch().lower()
        if answer == 'y':
            sys.stdout.write(answer+'\n')
            return True
        elif answer == 'n':
            sys.stdout.write(answer+'\n')
            return False
        elif answer == 's':
            sys.stdout.write(answer)
            show_callback(pid)
        else:
            sys.exit(1)


def open_explorer():
    """Open explorer.exe process."""
    # Just opening 'explorer.exe' merely launches a Windows Explorer window,
    # which is not the desired effect here.
    log.info("Re-opening %s" % EXPLORER)
    windir = os.environ['windir']
    subprocess.Popen(os.path.join(windir, EXPLORER))


def main():
    arguments = docopt(__doc__, options_first=True)
    name = arguments['<name>']
    quiet = arguments['-y']
    hotkeys = arguments['-s']

    activatePID = partial(_activatePID, enable_hotkeys=hotkeys)
    name_abspath = abspath(name)
    log.debug("name_abspath=%s" % name_abspath)
    locks = find_locks(name_abspath)
    if not locks:
        log.info("Nothing locking %s" % name_abspath)
        sys.exit(0)

    for process, pid in locks:
        if not quiet:
            do_kill = query(process, pid, show_callback=activatePID)
            if do_kill is not True:
                continue
        kill(pid)
        log.debug("Killed PID %s" % pid)
        if process == EXPLORER:
            time.sleep(0.1)
            open_explorer()

if __name__ == '__main__':
    main()

EDIT: As per @janos request, the code to be reviewed can be found below. I am not sure if using logging solely for the purpose of adding a prefix to my output messages was a good idea, but it felt very redundant to add it to every print. Making a function that wraps print made the code look weird.

Also, I'm at a loss as to how to test this. I've added tests for abspath function (see repo), but I'm having trouble writing tests for the other functions.

"""Dammit let me delete this file!

Usage:
    dammit [-y | -s] <name>
    dammit (-h)

Options:
-y        Kill without permission.
-s        Enable hotkey
-h        Show this screen.

"""
from __future__ import print_function

from docopt import docopt
from functools import partial
import logging
import msvcrt
import os
import posixpath
import re
import subprocess
import sys
import time

log = logging.getLogger()
log.setLevel(logging.INFO)
ch = logging.StreamHandler()
ch.setLevel(logging.INFO)
PREFIX = 'dammit:'
formatter = logging.Formatter(PREFIX + ' %(message)s')
ch.setFormatter(formatter)
log.addHandler(ch)

CYGPATH = ['C:\\GnuNT\\bin\\cygpath.exe']
HANDLE = ["handle64.exe"]
# handle.exe provides way to close specified handle, which would be preferable,
# but this requires administrator rights :(
KILL = ["TASKKILL", "/f", "/pid"]
ACTIVATE = ["activatePID.exe"]
EXPLORER = "explorer.exe"
CANT_EXECUTE = """Could not execute command: %s"""


def abspath(path):
    """Return the absolute path of given `path`.

    Taken to be relative from current working directory if input is not an
    absolute path.

    Args:
        path (str) : Pathname to absolutize.

    Notes:
        Should work for POSIX and Windows paths to accomodate
        different shells.
    """
    if posixpath.isabs(path):  # Starts with "/"
        if path[1:9] != 'cygdrive':
            path = posixpath.join('/cygdrive', path[1:])
        proc = subprocess.Popen(CYGPATH + ['-w', path],
                                stdout=subprocess.PIPE)
        return proc.stdout.read().rstrip()  # Remove trailing newline
    return os.path.abspath(path)


def find_locks(name):
    """Find which processes currently have a particular file/directory open.

    Args:
        name (str) : Name of the file which is locked.

    Returns:
        List of (process, PID)-pairs which are accessing the file.
    """
    try:
        output = subprocess.check_output(HANDLE + [name])
    except WindowsError:
        log.info(CANT_EXECUTE % HANDLE)
        sys.exit(1)
    except subprocess.CalledProcessError:
        return None
    pattern = r"([^\s]+)(?:\s+)pid: ([0-9]{4})"
    return frozenset(re.findall(pattern, output))


def kill(pid):
    """Kill process with given PID

    Args:
        pid (str) : PID to kill
    """
    try:
        with open(os.devnull, 'w') as nul:
            subprocess.Popen(KILL + [str(pid)], stdout=nul, stderr=nul)
    except WindowsError:
        log.info(CANT_EXECUTE % KILL)
        sys.exit(1)
    return


def _activatePID(pid, enable_hotkeys):
    """Try to activate window associated with given process ID

    Args:
        pid (str) : PID of process to activate.
        enable_hotkeys (bool) : If True, Autohotkey will send "y" and "n"
            key presses to the prompt upon activating requested window.
    """
    try:
        subprocess.Popen(ACTIVATE + [pid, str(enable_hotkeys)])
    except WindowsError:
        log.info(CANT_EXECUTE % ACTIVATE)
    return


def query(process, pid, show_callback):
    """Query user to kill or show process.

    No invalid answers allowed.

    Args:
        process (str) : Process name
        pid (str) : Process PID
        show_callback (func(x)) : Ran when user answers "show"


    Answers:
        y : Kill process
        n : Do not kill process
        s : Try to show (activate) window linked to the process.

    Answering "show" does not exit the prompt.

    Returns:
        bool: True if yes, False if no.
    """
    question = " Kill process %s with PID %s [y/n/s]? " % (process, pid)
    sys.stdout.write(PREFIX + question)
    while True:
        answer = msvcrt.getch().lower()
        if answer == 'y':
            sys.stdout.write(answer+'\n')
            return True
        elif answer == 'n':
            sys.stdout.write(answer+'\n')
            return False
        elif answer == 's':
            sys.stdout.write(answer)
            show_callback(pid)
        else:
            sys.exit(1)


def open_explorer():
    """Open explorer.exe process."""
    # Just opening 'explorer.exe' merely launches a Windows Explorer window,
    # which is not the desired effect here.
    log.info("Re-opening %s" % EXPLORER)
    windir = os.environ['windir']
    subprocess.Popen(os.path.join(windir, EXPLORER))


def main():
    arguments = docopt(__doc__, options_first=True)
    name = arguments['<name>']
    quiet = arguments['-y']
    hotkeys = arguments['-s']

    activatePID = partial(_activatePID, enable_hotkeys=hotkeys)
    name_abspath = abspath(name)
    log.debug("name_abspath=%s" % name_abspath)
    locks = find_locks(name_abspath)
    if not locks:
        log.info("Nothing locking %s" % name_abspath)
        sys.exit(0)

    for process, pid in locks:
        if not quiet:
            do_kill = query(process, pid, show_callback=activatePID)
            if do_kill is not True:
                continue
        kill(pid)
        log.debug("Killed PID %s" % pid)
        if process == EXPLORER:
            time.sleep(0.1)
            open_explorer()

if __name__ == '__main__':
    main()
Source Link
selimb
  • 173
  • 4
Loading