[Lldb-commits] [PATCH] D64251: Don't depend on psutil on AIX

[Dan Liew via Phabricator via lldb-commits]

delcypher added inline comments.


================
Comment at: llvm/utils/lit/lit/LitConfig.py:93
             # See lit.util.killProcessAndChildren()
-            try:
-                import psutil  # noqa: F401
-            except ImportError:
-                self.fatal("Setting a timeout per test requires the"
-                           " Python psutil module but it could not be"
-                           " found. Try installing it via pip or via"
-                           " your operating system's package manager.")
+            if platform.system() != 'AIX':
+                try:
----------------
If we're going to start specializing how we support this feature on different problems then I think we need to refactor this so we don't have a platform specific code littered throughout the lit code base.

There probably should be a function that determines if `killProcessAndChildren()` is supported. Here's a sketch

```lang=python
def killProcessAndChildrenIsSupported():
   """
      Returns a tuple (<supported> , <error message>)
      where 
       `<supported>` is True if `killProcessAndChildren()` is supported on the current host, returns False otherwise.
      `<error message>` is an empty string if `<supported>` is True, otherwise is contains a string describing why the function is not supported.
    """
    if platform.system() == 'AIX':
       return (True, "")
     try:
       import psutil
       return (True, "")
     except ImportError:
        return (False, "Python psutil module could not be found")
```

This `killProcessAndChildrenIsSupported()` function can then called in any place where we currently try to do `import psutil`. This hides the platform specific logic that you are trying to introduce here.


================
Comment at: llvm/utils/lit/lit/util.py:440
+        from subprocess import call
+        call('kill -kill $(ps -o pid= -L{})'.format(pid), shell=True)
+    else:
----------------
This `from` statement seems unnecessary given that we already have `subprocess` imported and we just use `call()` once in this context. I'd rather this was just

```
subprocess.call('kill -kill $(ps -o pid= -L{})'.format(pid), shell=True)
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64251/new/

https://reviews.llvm.org/D64251