This is an archive of the discontinued LLVM Phabricator instance.

Disable update_cc_test_checks.py tests in stand-alone builds
ClosedPublic

Authored by tstellar on Apr 18 2022, 4:51 PM.

Details

Summary

The script is located in the llvm/ sub-directory, so it is not available
for when doing a stand-alone build.

See https://discourse.llvm.org/t/rfc-stand-alone-build-support/61291

Diff Detail

Event Timeline

tstellar created this revision.Apr 18 2022, 4:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 4:51 PM
Herald added a subscriber: mgorny. · View Herald Transcript
tstellar requested review of this revision.Apr 18 2022, 4:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 4:51 PM
rovka added a subscriber: rovka.Apr 20 2022, 12:58 AM

I just have a couple of nits, I'll leave it to the clang devs to properly review this.

clang/test/CMakeLists.txt
17

Nit: These seem to be sorted alphabetically.

clang/test/utils/update_cc_test_checks/lit.local.cfg
17
tstellar updated this revision to Diff 424040.Apr 20 2022, 2:54 PM

Address review comments.

tstellar marked 2 inline comments as done.Apr 20 2022, 2:55 PM
tstellar edited the summary of this revision. (Show Details)Jul 11 2022, 9:22 PM
tstellar added reviewers: h-vetinari, sammccall.

Just comments, looks OK otherwise.

clang/test/CMakeLists.txt
6

OT for this PR, but it would be nice to eventually rename this to CLANG_BUILD_STANDALONE in accordance with the present tense used for other variables.

clang/test/lit.site.cfg.py.in
40

I'd use one spelling of standalone / stand-alone, and I think in this case it's probably the env var that wins?

clang/test/utils/update_cc_test_checks/lit.local.cfg
19

I couldn't tell from the diff where this is used (though admittedly I hardly know the LLVM infra). I also don't see it in [[ https://github.com/llvm/llvm-project/blob/main/clang/test/lit.cfg.py | lit.cfg.py ]], the respective [[ https://github.com/llvm/llvm-project/blob/main/clang/test/CMakeLists.txt | CMakeLists.txt ]] or [[ https://github.com/llvm/llvm-project/blob/llvmorg-14.0.6/llvm/cmake/modules/AddLLVM.cmake#L1616 | configure_lit_site_cfg ]]

I trust that this does what's intended.

tstellar updated this revision to Diff 449681.Aug 3 2022, 9:15 AM
tstellar marked an inline comment as done.

Fix option spelling.

clang/test/utils/update_cc_test_checks/lit.local.cfg
19

The lit tool uses this to decide whether or not to run the tests in the directory.

@h-vetinari Do these updates look OK to you?

This revision was not accepted when it landed; it landed in state Needs Review.Aug 11 2022, 8:56 PM
This revision was automatically updated to reflect the committed changes.