diff options
| author | Peter Dillinger <peterd@meta.com> | 2024-06-14 16:32:28 -0700 |
|---|---|---|
| committer | Facebook GitHub Bot <facebook-github-bot@users.noreply.github.com> | 2024-06-14 16:32:28 -0700 |
| commit | d6979bda409504c98261426b27220d3f48eabbac (patch) | |
| tree | 2e239acba40c362fe0dd34b2f29eb091ccd01cd7 | |
| parent | 0ab60b8a8c9a162f30eeb163548003e6e702eb1a (diff) | |
Verify public headers do not reference internal ones (#12774)
Summary:
This is not currently caught by our public CI so adding a form of this check to `make check-headers`, which is part of the build-linux-unity-and-headers GHA job.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/12774
Test Plan: manually added a violation, which was caught. Also caught an existing trivial violation (fixed). CI will verify it plays nice with GHA.
Reviewed By: jowlyzhang
Differential Revision: D58616601
Pulled By: pdillinger
fbshipit-source-id: e656ce82709660c088a3d3a5e41dd07655cb40e0
| -rw-r--r-- | Makefile | 23 | ||||
| -rw-r--r-- | include/rocksdb/utilities/customizable_util.h | 1 |
2 files changed, 17 insertions, 7 deletions
@@ -636,26 +636,37 @@ TESTS = $(patsubst %.cc, %, $(notdir $(TEST_MAIN_SOURCES))) TESTS += $(patsubst %.c, %, $(notdir $(TEST_MAIN_SOURCES_C))) TESTS += $(PLUGIN_TESTS) -# `make check-headers` to very that each header file includes its own -# dependencies +# `make check-headers` to verify that each header file includes its own deps +# and that public headers do not depend on internal headers ifneq ($(filter check-headers, $(MAKECMDGOALS)),) # TODO: add/support JNI headers DEV_HEADER_DIRS := $(sort include/ $(dir $(ALL_SOURCES))) # Some headers like in port/ are platform-specific - DEV_HEADERS := $(shell $(FIND) $(DEV_HEADER_DIRS) -type f -name '*.h' | grep -E -v 'port/|plugin/|lua/|range_tree/') + DEV_HEADERS_TO_CHECK := $(shell $(FIND) $(DEV_HEADER_DIRS) -type f -name '*.h' | grep -E -v 'port/|plugin/|lua/|range_tree/') + PUBLIC_HEADERS_TO_CHECK := $(shell $(FIND) include/ -type f -name '*.h' | grep -E -v 'lua/') else - DEV_HEADERS := + DEV_HEADERS_TO_CHECK := + PUBLIC_HEADERS_TO_CHECK := endif -HEADER_OK_FILES = $(patsubst %.h, %.h.ok, $(DEV_HEADERS)) +HEADER_OK_FILES = $(patsubst %.h, %.h.ok, $(DEV_HEADERS_TO_CHECK)) \ + $(patsubst %.h, %.h.pub, $(PUBLIC_HEADERS_TO_CHECK)) AM_V_CCH = $(am__v_CCH_$(V)) am__v_CCH_ = $(am__v_CCH_$(AM_DEFAULT_VERBOSITY)) am__v_CCH_0 = @echo " CC.h " $<; am__v_CCH_1 = +# verify headers include their own dependencies, under dev build settings %.h.ok: %.h # .h.ok not actually created, so re-checked on each invocation # -DROCKSDB_NAMESPACE=42 ensures the namespace header is included - $(AM_V_CCH) echo '#include "$<"' | $(CXX) $(CXXFLAGS) -DROCKSDB_NAMESPACE=42 -x c++ -c - -o /dev/null + $(AM_V_CCH) echo '#include "$<"' | $(CXX) $(CXXFLAGS) \ + -DROCKSDB_NAMESPACE=42 -x c++ -c - -o /dev/null + +# verify public headers do not depend on internal headers, under typical +# user build settings +%.h.pub: %.h # .h.pub not actually created, so re-checked on each invocation + $(AM_V_CCH) cd include/ && echo '#include "$(patsubst include/%,%,$<)"' | \ + $(CXX) -I. -DROCKSDB_NAMESPACE=42 -x c++ -c - -o /dev/null check-headers: $(HEADER_OK_FILES) diff --git a/include/rocksdb/utilities/customizable_util.h b/include/rocksdb/utilities/customizable_util.h index adf254054..fe160f17e 100644 --- a/include/rocksdb/utilities/customizable_util.h +++ b/include/rocksdb/utilities/customizable_util.h @@ -16,7 +16,6 @@ #include <memory> #include <unordered_map> -#include "options/configurable_helper.h" #include "rocksdb/convenience.h" #include "rocksdb/customizable.h" #include "rocksdb/status.h" |