tahonermann added inline comments.
================ Comment at: clang/www/hacking.html:295-296 + directory will cause the update of the diff to start a CI run. This dummy + file will also add the libc++ group to the list of reviewers. The status of + the build will be available in Phabricator.</p> + ---------------- ldionne wrote: > Mordante wrote: > > philnik wrote: > > > aaron.ballman wrote: > > > > I am guessing the dummy file should then be removed before landing the > > > > commit and we should document that? > > > > > > > > (FWIW, adding a dummy file feels really unclean as a design -- it's > > > > mysterious behavior for new contributors, which the documentation helps > > > > with a bit, but also it's a risk for checking in files that are never > > > > intended to be in the tree. If there's a way to improve this somehow so > > > > we don't need to trick the precommit CI into running, that would be > > > > really nice and totally outside of the scope of this review.) > > > It would be great if just the bootstrapping build could be run on all > > > clang reviews. That should show most problems with changes in clang. If > > > there are any problems in libc++, fixing them would result in a full CI > > > run. Having libc++ run against the patch would probably also be awesome > > > as a smoke test for any clang changes. > > +1 to what @ldionne said. > > > > One of the advantages would be that it's possible to test Clang against the > > libc++ code base using different language versions, by defining multiple > > test runs using different language versions. (Slightly related to > > https://discourse.llvm.org/t/lit-run-a-run-line-multiple-times-with-different-replacements/64932) > > @aaron.ballman is looking at extending the Clang pre-commit CI something > > worth looking into? > > > > > I agree. Let's: > > 1. Setup a `buildkite-pipeline.yml` file that controls the jobs done for > Clang. I think right now this is hardcoded somewhere in > https://github.com/google/llvm-premerge-checks > 2. Simply add `LLVM_ENABLE_RUNTIMES=libcxx` to the config that Clang uses so > they'll automatically build libc++ with the Clang that was just built anyway. > That would run on Clang's own agents that are used for Clang tests, not on > the libc++ CI agents. We can adjust if running the libc++ tests causes too > much strain on the infrastructure running Clang CI. > > That way, a basic bootstrapping build of libc++ will be run on every Clang > change, and TBH I think all of this added documentation for Clang folks to > test libc++ changes can simply go away. There really wouldn't be a reason to > ever add a `.DELETE_ME` file to libc++ for a Clang patch anymore. This sounds like a good approach to me. This might be a bit tangential, but if `libcxx` is present in `LLVM_ENABLE_RUNTIMES`, could we also have the `check-all` target depend on `check-libcxx`? That would help ensure that developers run libc++ tests locally before they hit the CI infrastructure. Likewise for `compiler-rt` and `libcxxabi` and the `check-runtimes` target. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133249/new/ https://reviews.llvm.org/D133249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
