[bazel] Re-implement IWYU enforcement

Instead of having a feature that a subpackage cc_library
would set (because the subpackage cc_libraries went away in
review.skia.org/543977), we now have a list of directories
for which all descendent files will have IWYU run on them.

As before, we have an overarching feature "skia_enforce_iwyu"
that enables this check because of the extra compilation
overhead it incurs.

We have decided to enforce IWYU on debug builds instead of
release builds because we have some code (e.g. that in
SkDEBUGCODE or SkASSERT) which is only compiled and executed
in a debug build, but we don't want to have
#if defined(SK_DEBUG)
all over the place. We make the assumption that the includes
needed to compile in debug mode are the superset of the
includes necessary for release and other modes.

Change-Id: I10254fcc162627c20eb89959e06417effa3cc396
Bug: skia:12541 skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546607
Reviewed-by: Ben Wagner <bungeman@google.com>

bazel/buildrc

@@ -37,3 +37,8 @@ build:debug --compilation_mode=dbg
 build:ck_release --config=release --spawn_strategy=local
 build:ck_debug --config=debug --spawn_strategy=local
 
+# We only want to enforce IWYU on debug builds because we have some things that are only
+# necessary to include in debug mode (e.g. SkDEBUGCODE), but very rarely something that is
+# only needed in release mode. Thus our C++ debug includes should be a superset of the
+# release includes.
+build:enforce_iwyu --features=skia_enforce_iwyu --cc_output_directory_tag=iwyu --compilation_mode=dbg

bazel/macros.bzl

@@ -64,12 +64,6 @@ def select_multi(values_map, default):
         })
     return rv
 
-# buildifier: disable=unnamed-macro
-# buildifier: disable=native-package
-def enforce_iwyu_on_package():
-    """A self-annotating macro to set force_iwyu = True on all rules in this package."""
-    native.package(features = ["skia_opt_file_into_iwyu"])
-
 # buildifier: disable=unnamed-macro
 def cc_library(**kwargs):
     """A shim around cc_library that lets us tweak settings for G3 if necessary."""

toolchain/linux_amd64_toolchain_config.bzl

@@ -15,7 +15,6 @@ load(
     "@bazel_tools//tools/cpp:cc_toolchain_config_lib.bzl",
     "action_config",
     "feature",
-    "feature_set",
     "flag_group",
     "flag_set",
     "tool",
@@ -347,10 +346,13 @@ def _make_iwyu_flags():
             flag_group(
                 flags = [
                     # This define does not impact compilation, but it acts as a signal to the
-                    # clang_trampoline.sh whether check the file with include-what-you-use
+                    # clang_trampoline.sh whether to maybe check the file with include-what-you-use
                     # A define was chosen because it is ignored by clang and IWYU, but can be
                     # easily found with bash.
-                    "-DSKIA_ENFORCE_IWYU_FOR_THIS_FILE",
+                    # The clang_trampoline.sh file has a list of allowed subdirectories for which
+                    # IWYU should be enforced, allowing us to slowly opt more and more directories
+                    # in over time.
+                    "-DSKIA_ENFORCE_IWYU",
                 ],
             ),
         ],
@@ -358,24 +360,10 @@ def _make_iwyu_flags():
 
     return [
         feature(
-            # The IWYU checks can add some overhead to the build (1-5 seconds per file), so we only
-            # want to run them sometimes. By adding --feature skia_enforce_iwyu to the Bazel
-            # command, this will turn on the checking (for all files that have not been opted-out).
             "skia_enforce_iwyu",
             enabled = False,
-        ),
-        feature(
-            "skia_opt_file_into_iwyu",
-            enabled = False,
             flag_sets = [
                 opt_file_into_iwyu,
             ],
-            # If the skia_enforce_iwyu features is not enabled (e.g. globally via a CLI flag), we
-            # will not run the IWYU analysis on any files.
-            requires = [
-                feature_set(features = [
-                    "skia_enforce_iwyu",
-                ]),
-            ],
         ),
     ]

toolchain/linux_trampolines/clang_trampoline_linux.sh

@@ -6,19 +6,43 @@
 
 export LD_LIBRARY_PATH="external/clang_linux_amd64/usr/lib/x86_64-linux-gnu:external/clang_linux_amd64/usr/lib/llvm-13/lib"
 
-# If compilation fails, we want to exit right away
-set -e
-# We only want to run include-what-you-use if SKIA_ENFORCE_IWYU_FOR_THIS_FILE is in the arguments
-# passed in (i.e. the "skia_opt_file_into_iwyu" feature is enabled) and we are not linking
+# We only want to run include-what-you-use if DSKIA_ENFORCE_IWYU is in the arguments
+# passed in (i.e. the "skia_enforce_iwyu" feature is enabled) and we are not linking
 # (as detected by the presence of -fuse-ld).
-if [[ "$@" != *SKIA_ENFORCE_IWYU_FOR_THIS_FILE* || "$@" == *use-ld* ]]; then
+if [[ "$@" != *DSKIA_ENFORCE_IWYU* || "$@" == *use-ld* ]]; then
+  external/clang_linux_amd64/bin/clang $@
+  exit 0
+fi
+
+supported_files_or_dirs=(
+  "experimental/bazel_test/"
+)
+
+function opted_in_to_IWYU_checks() {
+  # Need [@] for entire list: https://stackoverflow.com/a/46137325
+  for path in ${supported_files_or_dirs[@]}; do
+    if [[ $1 == *"-c $path"* ]]; then
+      echo $path
+      return 0
+    fi
+  done
+  echo ""
+  return 0
+}
+
+# We want to concatenate all args into a string so we can do some
+# string matching in the opted_in_to_IWYU_checks function.
+# https://unix.stackexchange.com/a/197794
+opt_in=$(opted_in_to_IWYU_checks "'$*'")
+if [[ -z $opt_in ]]; then
   external/clang_linux_amd64/bin/clang $@
   exit 0
 else
   # Now try to compile with Clang, and then verify with IWYU
   external/clang_linux_amd64/bin/clang $@
-  # IWYU always returns a non-zero code because it doesn't produce the .o file (that's why
+  # IWYU always [1] returns a non-zero code because it doesn't produce the .o file (that's why
   # we ran Clang first). As such, we do not want bash to fail after running IWYU.
+  # [1] Until v0.18 at least
   set +e
   # Get absolute path to the mapping file because resolving the relative path is tricky, given
   # how Bazel locates the toolchain files.
@@ -26,7 +50,8 @@ else
   # IWYU always outputs something to stderr, which can be noisy if everything is fixed.
   # Otherwise, we send the exact same arguments to include-what-you-use that we would for
   # regular compilation with clang.
-  # We always allow SkTypes.h because
+  # We always allow SkTypes.h because it sets some defines that later #ifdefs use and IWYU is
+  # not consistent with detecting that.
   external/clang_linux_amd64/usr/bin/include-what-you-use \
       -Xiwyu --keep="include/core/SkTypes.h" \
       -Xiwyu --mapping_file=$MAPPING_FILE $@ 2>/dev/null