[PATCH] D110839: [LNT] Added reading nm and objdump paths from env variables

Wed Oct 20 09:14:11 PDT 2021

thopre added a comment.

In D110839#3075336 <https://reviews.llvm.org/D110839#3075336>, @slydiman wrote:

> In D110839#3074994 <https://reviews.llvm.org/D110839#3074994>, @thopre wrote:
>
>> Please add some tests for the environment variables and possibly for the bug fixes depending on what they are. This should also be split into 4 patches, one for each items mentioned. They are all independent, even if you likely made them in one go to get Linux perf work for you. One important reason for splitting is if an issue is found with one of the items the whole patch might need to be reverted. It also allow for easier cherry-pick for people who maintain their own branch of LNT.
>>
>> Besides the missing tests, the patch LGTM and I'll be happy to approve it.
>
> Most changes in cPerf.cpp are related to the standalone mode which is not used in the production. This mode is designed only for debug purpose. It cannot break anything.

Still, the title does not reflect what is happening in the diff which is a clear sign the patch should be split and this is a completely independent change. If the fixes are also for standalone mode I'm happy for the patch to be split in 2 only: standalone+fixes and envvars+bincacheroot.

> BTW, I see that LNT tests did not execute automatically. Something is totally broken here around LNT tests and all LNT tests are useless for now. Who should we ping to fix this?

Whoever broke the tests. There's at least 3 failures:

- plistlib change I introduced seems to only work for Python3, I'll make a patch
- future needs to be installed for mypy to work (I'm guessing some recent Python version don't have it in the standard library anymore because it used to work) I've created https://reviews.llvm.org/D112148
- Something in runtest/test_suite.shtest

I think py27 fails because of plistlib change I introduced which only works for Python 3. I need to revisit that. Python3 fails for me because I think I'm using a too new version which seems to have dropped future.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110839