[libcxx-commits] [PATCH] D139545: [libc++][CI] Improves clang-tidy selection.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/utils/ci/run-buildbot:45-49
+CLANG_TIDY          The clang-tidy binary to use in the unit tests. When the
+                    value is omitted, it defaults to 'clang-tidy.exe' on Windows
+                    and 'clang-tidy' on other platforms. The unit test validate
+                    whether the executable can be used, if not the tests
+                    requiring clang-tidy are unsupported.
----------------
This can be removed.


================
Comment at: libcxx/utils/libcxx/test/features.py:155-156
           when=lambda cfg: runScriptExitCode(cfg, ['%{exec} bash -c \'bash --version\'']) != 0),
   Feature(name='has-clang-tidy',
           when=_hasSuitableClangTidy),
   Feature(name='has-clang-query',
----------------
I think I would dumb down the whole patch to this. It does mean that only folks with exactly `clang-tidy-15` will get those tests enabled, but it's already pretty much the status quo, and doing this avoids piping through the clang-tidy version across multiple layers like buildkite-pipeline.yml and CMake.

We should also do the same for `clang-query` in the same patch.

If we want to be more clever about finding `clang-tidy` (e.g. falling back on `clang-tidy --version >= 15`), we can do something similar to what we do with `getStdFlag` in `params.py` and basically do a linear search with various options.

Also, restricting the changes to this means that we avoid adding another dependency from the test suite onto CMake, which is good since we want to strive for the test suite being as freestanding as possible (within reason).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139545