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

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 8 13:02:38 PDT 2021


ldionne added a comment.

LGTM as-is, we can try to address the dirtying-working-directory issue by changing the scripts.



================
Comment at: libcxx/utils/ci/run-buildbot:144
+    # Check if the diffs are empty, fail otherwise.
+    ! grep -q '^--- a' ${BUILD_DIR}/generated_output.patch
+;;
----------------
Mordante wrote:
> ldionne wrote:
> > 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.
> Good to know. I wasn't aware the same checkout was reused by other jobs. If wanted I can add a `git reset --hard HEAD` after generating the diff.
Jobs do reuse the same checkout, but they use `git checkout --force <SHA>`, so it'll override any diff created by running this.

Don't do `git reset --hard HEAD` here, as this could cause people to lose work if they run this job locally. I run jobs locally often, not sure if others do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99862



More information about the libcxx-commits mailing list