[libcxx-commits] [PATCH] D116958: [libc++] Alphabetize CMakeLists.txt and module.modulemap; enforce in CI.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D116958#3237512 <https://reviews.llvm.org/D116958#3237512>, @arichardson wrote:

> In the past I have regularly been running libc++ tests with remote executors where the source code is not available on the system that runs the tests, so I'm almost certain this would fail. I haven't done this for a while so I can't remember if there is a "remote-executor" or similar lit feature that can be used for `UNSUPPORTED`.

This test is run only on the build host, not on the remote target. So this would work as long as the build host has Python (which it has to because it must run Lit).

This wouldn't work if we used `%{exec} %{python} ...`, which would try to run on the remote target.



================
Comment at: libcxx/test/libcxx/lint/lint_cmakelists.sh.py:6
+
+if __name__ == '__main__':
+    libcxx_test_libcxx_lint = os.path.dirname(os.path.abspath(__file__))
----------------
Is this really useful? IMO it just has the effect of indenting everything artificially.


================
Comment at: libcxx/test/libcxx/lint/lint_cmakelists.sh.py:7-10
+    libcxx_test_libcxx_lint = os.path.dirname(os.path.abspath(__file__))
+    libcxx = os.path.abspath(os.path.join(libcxx_test_libcxx_lint, '..', '..', '..'))
+    cmakelists_name = os.path.join(libcxx, 'include', 'CMakeLists.txt')
+    assert os.path.isfile(cmakelists_name)
----------------
arichardson wrote:
> I find python path manipulation a bit easier to read with `from pathlib import Path` and we depend on 3.5 (or 3.6?) now so it can be used.
Any reason why you're not inferring this from `%{libcxx}` instead? It would future proof this test in case it gets moved around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116958