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

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Apr 5 08:44:34 PDT 2021


Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.

LGTM % comment.



================
Comment at: libcxx/utils/ci/run-buildbot:137
+    clean
+    echo "+++ Checking the output of the generated scripts"
+    mkdir -p ${BUILD_DIR}
----------------
Nit: /generated scripts/generator scripts/
and maybe /output/outputs/



================
Comment at: libcxx/utils/ci/run-buildbot:142
+        | tee ${BUILD_DIR}/generate_feature_test_macro_components.patch
+    git reset --hard HEAD
+    python3 libcxx/utils/generate_header_tests.py
----------------
I suggest you remove this `git reset`, and do just one `git diff` at the end. I.e.,

```
python3 libcxx/utils/generate_feature_test_macro_components.py
python3 libcxx/utils/generate_header_tests.py
git diff \
        | tee ${BUILD_DIR}/check-generated-output.patch
! grep -q '^--- a' \
    ${BUILD_DIR}/check-generated-output.patch
```
This makes it a one-line diff to add a new generator script, as opposed to a 5-line diff (where one line is even separated from the other 4).

Personally I'm willing for you to make this change and //not// re-run all your tedious testing, if you don't want to expend that effort a second time. :)  I will undoubtedly make my suggested change in D99309 if you don't make it here.


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