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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Thanks for the suggestions.



================
Comment at: libcxx/test/configs/cmake-bridge.cfg.in:33
 config.substitutions.append(('%{executor}', '@LIBCXX_EXECUTOR@'))
+config.substitutions.append(('%{clang-tidy}', shlex.quote('@LIBCXX_TEST_CLANG_TIDY@')))
----------------
ldionne wrote:
> I would like to keep this to the smallest set of substitutions that we need to make the test suite work, so I am not sure about this approach. I'll need to take another look at this when I have more time.
As discussed yesterday with @ldionne we go with a different approach so this change is undone.


================
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',
----------------
ldionne wrote:
> 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).
This didn't work as suggested, I used the same syntax as the bash test above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139545