[compiler-rt] [compiler-rt][tests] Fix env command not found errors with lit internal shell (PR #105879)
Paul Kirth via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 23 12:51:02 PDT 2024
https://github.com/ilovepi requested changes to this pull request.
I'm not unsupportive of the goal of the patch, but I think there is a misunderstanding w.r.t. what the test was trying to do. Prefixing the use of the environment variable w/ `env` should be fine, but there seem to be other rather baffling changes, like adding directory names, and storing additional output to a temp file that is used w/ `FileCheck`. It doesn't make sense to pass additional output to `FileCheck`, if there aren't additional checks for that input.
The description also mentions not directing to `/dev/null`, but that part wasn't changed (nor do I think it should be).
Can you clarify why some of these changes/claims were made. I'd think this patch would only require adding an `env` prefix to line 17, so helping us understand why the rest is required or a good idea would go a long way in the review process..
https://github.com/llvm/llvm-project/pull/105879
More information about the llvm-commits
mailing list