delcypher added inline comments.
================
Comment at: libcxx/utils/libcxx/util.py:256
"""
- import psutil
- try:
- psutilProc = psutil.Process(pid)
- # Handle the different psutil API versions
+ if platform.system() == 'AIX':
+ subprocess.call('kill -kill $(ps -o pid= -L{})'.format(pid),
shell=True)
----------------
Wait what... why is this duplicated in `libcxx/utils/libcxx/util.py`?
================
Comment at: lldb/lit/lit.cfg.py:81
+# lit complains if the value is set but it is not supported.
+if lit_config.killProcessAndChildrenIsSupported:
lit_config.maxIndividualTestTime = 600
----------------
Shouldn't this be
```lang=python
if lit_config.killProcessAndChildrenIsSupported():
```
?
================
Comment at: llvm/utils/lit/lit/LitConfig.py:80
+ def killProcessAndChildrenIsSupported(self):
+ """
----------------
I'd prefer if this code lived in `llvm/utils/lit/lit/util.py` next to
`killProcessAndChildren()` given that this function describes if that function
works.
I realise you need to use `lit_config.killProcessAndChildrenIsSupported()` from
inside a lit config but I'd suggest you add another level of indirection to
hide this implementation detail so that the API of `LitConfig` doesn't leak it.
```lang=python
class LitConfig(object):
...
@property
def maxIndividualTestTimeIsSupported(self):
"""
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.
"""
return lit.util.killProcessAndChildrenIsSupported()
```
Alternatively you could change the implementation of
`LitObject.maxIndividualTestTime` setter to throw a custom exception if an
attempt is made to set the timeout when the platform doesn't support it. This
custom exception would store the error message so it could be programmatically
accessed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64251/new/
https://reviews.llvm.org/D64251
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
