[libcxx-commits] [PATCH] D99862: [libc++] [CI] Validate the output of the generated scripts.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

Please wait for CI to finish before you commit this :-). Thanks!



================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:42
+          limit: 2
+
   - label: "C++03"
----------------
Mordante wrote:
> curdeius wrote:
> > Maybe we want to add a `wait` here, so that the CI doesn't run for nothing when this tests fails. @ldionne, WDYT?
> Good point! I think that makes sense, especially since our CI runs can take a long time.
Yeah, I guess I'm fine with adding a `wait` here. We can tweak it if we see it doesn't interact well with the pipeline.


================
Comment at: libcxx/utils/ci/run-buildbot:144
+    # Check if the diffs are empty, fail otherwise.
+    ! grep -q '^--- a' ${BUILD_DIR}/generated_output.patch
+;;
----------------
It's annoying that we'll be modifying our git index just to run this test, i.e. if it generates changes, then those changes will "pollute" the working directory.

I think the cleanest approach to this would be to change the various header/test generation files to behave more like `clang-format` and to produce a diff that we can then apply instead of making the changes in place, but that would be more work indeed. That way, we could run this test without modifying our local state.

That's definitely not blocking for this review, but something to keep in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99862