[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

Thu Sep 8 12:16:13 PDT 2022

ldionne 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>
+
----------------
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.
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249