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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL366912: [AIX][lit] Dont depend on psutil on AIX (authored by daltenty, committed by ). Changed prior to commit:

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

delcypher accepted this revision. delcypher added a comment. LGTM. @davide While I agree with sentiment of fixing psutil, the use of psutil was never meant to be permanent. As the owner of this code I don't mind making accommodations for other platforms provided we keep implementation details

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

daltenty updated this revision to Diff 211104. daltenty added a comment. - Fix typo in comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64251/new/ https://reviews.llvm.org/D64251 Files: libcxx/utils/libcxx/util.py lldb/lit/lit.cfg.py

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

delcypher added a comment. @daltenty Other than the minor nit, LGTM. Comment at: llvm/utils/lit/lit/util.py:449 +(recursively). It is currently implemented using the psutil module on some +plaftorms which provides a simple platform neutral implementation.

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

daltenty updated this revision to Diff 209621. daltenty added a comment. - Add a space to warning in LLDB lit config Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64251/new/ https://reviews.llvm.org/D64251 Files: libcxx/utils/libcxx/util.py

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

daltenty updated this revision to Diff 209619. daltenty marked an inline comment as done. daltenty added a comment. - Address review comments round 3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64251/new/ https://reviews.llvm.org/D64251 Files:

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

daltenty marked 6 inline comments as done. daltenty added inline comments. Comment at: llvm/utils/lit/lit/util.py:426 +def killProcessAndChildrenIsSupported(llvm_config): +""" delcypher wrote: > I don't really like how we're now coupling this function with

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

delcypher added inline comments. Comment at: llvm/utils/lit/lit/LitConfig.py:81 +@property +def killProcessAndChildrenIsSupported(self): +""" Sorry to be pedantic but this (`LitConfig.killProcessAndChildrenIsSupported`) should be called

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

daltenty updated this revision to Diff 209409. daltenty added a comment. - Address review comments round 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64251/new/ https://reviews.llvm.org/D64251 Files: libcxx/utils/libcxx/util.py

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

daltenty added a comment. In D64251#1577374 , @davide wrote: > This is adding a fair amount of complexity on something that just works fine > on basically every platform but AIX. > If AIX has issue with `psutil`, maybe the fix should be submitted to >

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

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This is adding a fair amount of complexity on something that just works fine on basically every platform but AIX. If AIX has issue with `psutil`, maybe the fix should be submitted to

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

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

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

daltenty updated this revision to Diff 208805. daltenty added a comment. - Use subprocess.call instead of importing it - Create killProcessAndChildrenIsSupported check and rewrite things to use it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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

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

delcypher added a comment. @daltenty The "This removes our dependency on psutil." text sounds broader than it actually is. Looking at the implementation the removal of the dependency is only for AIX. All other platforms still depends on the `psutil` module. I think the commit message should

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

hubert.reinterpretcast added inline comments. Comment at: llvm/utils/lit/lit/LitConfig.py:98 +self.fatal("Setting a timeout per test requires the" +" Python psutil module but it could not be" +" found.

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

daltenty created this revision. Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, christof, delcypher. Herald added projects: LLDB, libc++, LLVM. On AIX psutil can run into problems with permissions to read the process tree, which causes problems for python tests. This patch