zero9178 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> + ---------------- tahonermann wrote: > 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. This is currently being implemented by https://reviews.llvm.org/D132438 and landed before but had to be reverted unfortunately. 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
