[libcxx-commits] [libcxx] e880c19 - [libc++] [CI] Fail if the headers contain cyclic dependencies.

[Arthur O'Dwyer via libcxx-commits]

Author: Arthur O'Dwyer
Date: 2021-04-18T12:58:04-04:00
New Revision: e880c19c6af3078d42e74543bb4690cbefbff4dd

URL: https://github.com/llvm/llvm-project/commit/e880c19c6af3078d42e74543bb4690cbefbff4dd
DIFF: https://github.com/llvm/llvm-project/commit/e880c19c6af3078d42e74543bb4690cbefbff4dd.diff

LOG: [libc++] [CI] Fail if the headers contain cyclic dependencies.

Since we have a tool to detect cycles now; and since we're entering
a phase where people can easily introduce cycles by accident (D100682)
or by request (D90999), I think it's increasingly important to shift
the burden of detecting these cycles onto the buildbot instead of
the poor human reviewer.

Also, grep for non-ASCII characters (such as U+200B and U+00AD)
and hard tabs; don't let those get checked in.

Differential Review: https://reviews.llvm.org/D100703

Added: 
    

Modified: 
    libcxx/utils/ci/run-buildbot
    libcxx/utils/graph_header_deps.py

Removed: 
    


################################################################################
diff  --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index 78eb65a1cb64e..a327b938d7a0e 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -134,14 +134,19 @@ check-format)
 ;;
 check-generated-output)
     clean
-    echo "+++ Checking the output of the generated scripts"
+    echo "+++ Checking the output of the generator scripts"
     mkdir -p ${BUILD_DIR}
+    # Reject patches that introduce non-ASCII characters or hard tabs.
+    ! grep -rn '[^ -~]' libcxx/include/
+    # Reject patches that don't update the generated output correctly.
     python3 libcxx/utils/generate_feature_test_macro_components.py
     python3 libcxx/utils/generate_header_inclusion_tests.py
     python3 libcxx/utils/generate_header_tests.py
     git 
diff  | tee ${BUILD_DIR}/generated_output.patch
     # Check if the 
diff s are empty, fail otherwise.
     ! grep -q '^--- a' ${BUILD_DIR}/generated_output.patch
+    # Check that no dependency cycles have been introduced.
+    python3 libcxx/utils/graph_header_deps.py >/dev/null
 ;;
 generic-cxx03)
     export CC=clang

diff  --git a/libcxx/utils/graph_header_deps.py b/libcxx/utils/graph_header_deps.py
index 8a4840383dc95..1cf001564bc1b 100755
--- a/libcxx/utils/graph_header_deps.py
+++ b/libcxx/utils/graph_header_deps.py
@@ -10,6 +10,7 @@
 import argparse
 import os
 import re
+import sys
 
 
 def is_config_header(h):
@@ -60,7 +61,7 @@ def locate_header_file(h, paths):
     local_includes = []
     system_includes = []
     linecount = 0
-    with open(fname, 'r') as f:
+    with open(fname, 'r', encoding='utf-8') as f:
         for line in f.readlines():
             linecount += 1
             m = re.match(r'\s*#\s*include\s+"([^"]*)"', line)
@@ -116,13 +117,14 @@ def build_graph(roots, options):
     return graph
 
 
-def get_graphviz(graph, options):
+def get_friendly_id(fname):
+    i = fname.index('include/')
+    assert(i >= 0)
+    result = fname[i+8:]
+    return result
 
-    def get_friendly_id(fname):
-        i = fname.index('include/')
-        assert(i >= 0)
-        result = fname[i+8:]
-        return result
+
+def get_graphviz(graph, options):
 
     def get_decorators(fname, entry):
         result = ''
@@ -204,7 +206,9 @@ def should_keep(fname):
     for fname, entry in graph.items():
         for h in entry.includes:
             if transitively_includes(graph, h, fname):
-                print('Cycle detected between %s and %s' % (fname, h))
+                sys.stderr.write('Cycle detected between %s and %s\n' % (
+                    get_friendly_id(fname), get_friendly_id(h)
+                ))
                 no_cycles_detected = False
     assert no_cycles_detected