[libcxx-commits] [PATCH] D154987: [lit] Implement PYTHON directive and config.prologue

Thu Jul 27 09:05:37 PDT 2023

awarzynski added a comment.

In D154987#4531662 <https://reviews.llvm.org/D154987#4531662>, @jhenderson wrote:

> lit already has the ability to execute python scripts (call `%python`),

I forgot about that, thanks for the reminder! This means that "python super powers" that I was concerned about earlier are already available to us. Please ignore my earlier concerns regarding that part.

> I think it would be worth outlining in this patch why simply calling an external python script (which could actually be embedded within the file) is insufficient for the motivating case.

+1 Also, is this change meant to replace `%python`? That would be very nice and would also help maintain consistency within our tests. Otherwise, we will have "yet another way" to run Python.

> What lit internals do you mean? Do you mean it enables tests to directly access functions and symbols within lit's implementation? This patch specifically avoids that. Access to lit-provided features is via a carefully defined lit object in python.

@jdenny, thanks for clarifying.  I assumed (incorrectly) that the intention was to open-up all of LIT's internals to people writing tests. That's not the case. My bad for not reading the summary beforehand.

> The proposed support in this patch will help solve those problems in a way that I think is pretty straightforward yet powerful enough to be used to solve testing problems for other projects as well (such as the issue flang is having that @MaskRay pointed out).

IMHO, we should refrain from making assumptions about any potential refactoring that _might_ happen following this change. Unless there are volunteers to implement them. I am referring specifically to Flang:

- updating ~700 tests is not a small task,
- those tests were ported from Bash to Python (via `%python`) to enable better testing on Windows,
- I am not aware of any limitation of the current approach (with `%python`) that active Flang developers would like to have solved,
- those tests don't use FileCheck (previous discussion <https://discourse.llvm.org/t/rfc-port-test-modfile-sh-tests-in-flang-to-use-filecheck/55840> on that) or any other "typical" LLVM infra/tooling (i.e. there's relatively little alignment with other parts of LLVM).

It was one of my colleagues who ported those Flang tests from Bash to Python - that's how I know the context.

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

https://reviews.llvm.org/D154987