]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/commitdiff
Fix and detect headers with missing dependencies (#8893)
authorPeter Dillinger <peterd@fb.com>
Fri, 10 Sep 2021 16:59:05 +0000 (09:59 -0700)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Fri, 10 Sep 2021 17:00:26 +0000 (10:00 -0700)
Summary:
It's always annoying to find a header does not include its own
dependencies and only works when included after other includes. This
change adds `make check-headers` which validates that each header can
be included at the top of a file. Some headers are excluded e.g. because
of platform or external dependencies.

rocksdb_namespace.h had to be re-worked slightly to enable checking for
failure to include it. (ROCKSDB_NAMESPACE is a valid namespace name.)

Fixes mostly involve adding and cleaning up #includes, but for
FileTraceWriter, a constructor was out-of-lined to make a forward
declaration sufficient.

This check is not currently run with `make check` but is added to
CircleCI build-linux-unity since that one is already relatively fast.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8893

Test Plan: existing tests and resolving issues detected by new check

Reviewed By: mrambacher

Differential Revision: D30823300

Pulled By: pdillinger

fbshipit-source-id: 9fff223944994c83c105e2e6496d24845dc8e572

35 files changed:
.circleci/config.yml
Makefile
cache/cache_entry_stats.h
db/compaction/compaction_iteration_stats.h
db/job_context.h
db/pre_release_callback.h
db/read_callback.h
db/snapshot_impl.h
db_stress_tool/db_stress_compaction_filter.h
db_stress_tool/db_stress_listener.h
db_stress_tool/db_stress_table_properties_collector.h
include/rocksdb/functor_wrapper.h
include/rocksdb/rocksdb_namespace.h
include/rocksdb/thread_status.h
include/rocksdb/universal_compaction.h
include/rocksdb/utilities/ldb_cmd_execute_result.h
memory/memory_usage.h
table/block_based/block_type.h
table/table_properties_internal.h
table/table_reader_caller.h
util/cast_util.h
util/channel.h
util/crc32c_ppc.h
util/defer.h
util/duplicate_detector.h
util/fastrange.h
util/math.h
util/set_comparator.h
util/work_queue.h
util/xxph3.h
utilities/blob_db/blob_db_gc_stats.h
utilities/cassandra/cassandra_options.h
utilities/cassandra/serialize.h
utilities/trace/file_trace_reader_writer.cc
utilities/trace/file_trace_reader_writer.h

index 779a8ca07447b3d5ea3ce81f10149f6b921197ab..b395324967951f9f59ae250601e10b8446db5cac 100644 (file)
@@ -328,7 +328,7 @@ jobs:
       - run: (mkdir build && cd build && cmake -DWITH_GFLAGS=1 -DWITH_BENCHMARK=1 .. && make V=1 -j20 && ctest -j20 && make microbench) | .circleci/cat_ignore_eagain
       - post-steps
 
-  build-linux-unity:
+  build-linux-unity-and-headers:
     docker: # executor type
       - image: gcc:latest
     resource_class: large
@@ -336,6 +336,7 @@ jobs:
       - checkout # check out the code in the project directory
       - run: apt-get update -y && apt-get install -y libgflags-dev
       - run: TEST_TMPDIR=/dev/shm && make V=1 -j8 unity_test | .circleci/cat_ignore_eagain
+      - run: make V=1 -j8 -k check-headers | .circleci/cat_ignore_eagain # could be moved to a different build
       - post-steps
 
   build-linux-gcc-4_8-no_test_run:
@@ -730,9 +731,9 @@ workflows:
   build-linux-clang10-clang-analyze:
     jobs:
       - build-linux-clang10-clang-analyze
-  build-linux-unity:
+  build-linux-unity-and-headers:
     jobs:
-      - build-linux-unity
+      - build-linux-unity-and-headers
   build-windows-vs2019:
     jobs:
       - build-windows:
index b37e43e54db88e99190dbeedf1853fe74b0c5bc2..2748643782ef0ebd87aace1e08722945b94c2bdc 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -516,6 +516,29 @@ ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
        ALL_SOURCES += third-party/folly/folly/synchronization/test/DistributedMutexTest.cc
 endif
 
+# `make check-headers` to very that each header file includes its own
+# dependencies
+ifneq ($(filter check-headers, $(MAKECMDGOALS)),)
+# TODO: add/support JNI headers
+       DEV_HEADER_DIRS := $(sort include/ hdfs/ $(dir $(ALL_SOURCES)))
+# Some headers like in port/ are platform-specific
+       DEV_HEADERS := $(shell $(FIND) $(DEV_HEADER_DIRS) -type f -name '*.h' | egrep -v 'port/|plugin/|lua/|range_tree/|tools/rdb/db_wrapper.h|include/rocksdb/utilities/env_librados.h')
+else
+       DEV_HEADERS :=
+endif
+HEADER_OK_FILES = $(patsubst %.h, %.h.ok, $(DEV_HEADERS))
+
+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 =
+
+%.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
+
+check-headers: $(HEADER_OK_FILES)
+
 # options_settable_test doesn't pass with UBSAN as we use hack in the test
 ifdef COMPILE_WITH_UBSAN
         TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g')
@@ -716,7 +739,7 @@ endif  # PLATFORM_SHARED_EXT
 .PHONY: blackbox_crash_test check clean coverage crash_test ldb_tests package \
        release tags tags0 valgrind_check whitebox_crash_test format static_lib shared_lib all \
        dbg rocksdbjavastatic rocksdbjava gen-pc install install-static install-shared uninstall \
-       analyze tools tools_lib \
+       analyze tools tools_lib check-headers \
        blackbox_crash_test_with_atomic_flush whitebox_crash_test_with_atomic_flush  \
        blackbox_crash_test_with_txn whitebox_crash_test_with_txn \
        blackbox_crash_test_with_best_efforts_recovery \
@@ -2368,7 +2391,7 @@ build_subset_tests: $(ROCKSDBTESTS_SUBSET)
 
 # Remove the rules for which dependencies should not be generated and see if any are left.
 #If so, include the dependencies; if not, do not include the dependency files
-ROCKS_DEP_RULES=$(filter-out clean format check-format check-buck-targets check-sources jclean jtest package analyze tags rocksdbjavastatic% unity.% unity_test, $(MAKECMDGOALS))
+ROCKS_DEP_RULES=$(filter-out clean format check-format check-buck-targets check-headers check-sources jclean jtest package analyze tags rocksdbjavastatic% unity.% unity_test, $(MAKECMDGOALS))
 ifneq ("$(ROCKS_DEP_RULES)", "")
 -include $(DEPFILES)
 endif
index eb028a0877cd9af77a83eb2e7eb3c34fac56477a..cc452053d1b41fdc55ad474b142a4372590d2b9c 100644 (file)
@@ -15,6 +15,7 @@
 #include "rocksdb/cache.h"
 #include "rocksdb/status.h"
 #include "rocksdb/system_clock.h"
+#include "test_util/sync_point.h"
 #include "util/coding_lean.h"
 
 namespace ROCKSDB_NAMESPACE {
index 894aea1d96f9a6fe65d5b349a2f6f3b1ad2f850a..910c4469a69a877829ceaec56485e1fbc842e590 100644 (file)
@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include <cstdint>
+
 #include "rocksdb/rocksdb_namespace.h"
 
 struct CompactionIterationStats {
index 4b0a8edfc025ee77e9089721a02eb90b040ebc8b..8e191e31c4d57a18c4549aa086a8514f73a943bd 100644 (file)
@@ -12,8 +12,9 @@
 #include <string>
 #include <vector>
 
-#include "db/log_writer.h"
 #include "db/column_family.h"
+#include "db/log_writer.h"
+#include "db/version_set.h"
 
 namespace ROCKSDB_NAMESPACE {
 
index b74be9537563c2b67218d289f2aa853e6f6237ed..6b903948779e3d355436922bf42386fa645acf38 100644 (file)
@@ -6,11 +6,10 @@
 #pragma once
 
 #include "rocksdb/status.h"
+#include "rocksdb/types.h"
 
 namespace ROCKSDB_NAMESPACE {
 
-class DB;
-
 class PreReleaseCallback {
  public:
   virtual ~PreReleaseCallback() {}
index 8989c755abf9f26d24bc6901767b14b179587d78..c042352db0c2465b1912654c1977db4288ee6b42 100644 (file)
@@ -5,6 +5,7 @@
 
 #pragma once
 
+#include "db/dbformat.h"
 #include "rocksdb/types.h"
 
 namespace ROCKSDB_NAMESPACE {
index bfa44e3f53b12c9131f96a46c5662acb7e992871..9961bdd584fd09fa87ec91953b7c48b2ee0b35a2 100644 (file)
@@ -10,6 +10,7 @@
 #pragma once
 #include <vector>
 
+#include "db/dbformat.h"
 #include "rocksdb/db.h"
 
 namespace ROCKSDB_NAMESPACE {
index 60f8ac71d6e44389346b916c6ce77a53d86c6522..279c4452a8ea1088acb48de8e79c5d702c30f8d7 100644 (file)
@@ -9,6 +9,8 @@
 
 #pragma once
 
+#include "db_stress_tool/db_stress_common.h"
+#include "db_stress_tool/db_stress_shared_state.h"
 #include "rocksdb/compaction_filter.h"
 
 namespace ROCKSDB_NAMESPACE {
index 6985a77420da7a25b4fa246f5cfaefe87109acda..7fbab57e7119ce9a768ef0355dc25414c2e4a7f3 100644 (file)
@@ -3,11 +3,14 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
+#include "file/filename.h"
 #ifdef GFLAGS
 #pragma once
 
+#include "rocksdb/db.h"
 #include "rocksdb/listener.h"
 #include "util/gflags_compat.h"
+#include "util/random.h"
 
 DECLARE_int32(compact_files_one_in);
 
index 536d8a5f3ca6f7670e751865be03efa266d858a2..d1758cbb4cdcabaee202344a78c90a825d187589 100644 (file)
@@ -7,6 +7,7 @@
 
 #include "rocksdb/table.h"
 #include "util/gflags_compat.h"
+#include "util/random.h"
 
 DECLARE_int32(mark_for_compaction_one_file_in);
 
index c5f7414b1a8e4675b645b694fa9ea1291038ff38..17b021bf73b50d92a98a4911693af5b6e1ff4430 100644 (file)
@@ -5,6 +5,7 @@
 
 #pragma once
 
+#include <functional>
 #include <memory>
 #include <utility>
 
index e9f8620d06ecca860bfff0a6f8e143ceebc86fff..a339ec2aa5b934c947251d324152b480ca4e671a 100644 (file)
@@ -5,6 +5,12 @@
 
 #pragma once
 
+// For testing purposes
+#if ROCKSDB_NAMESPACE == 42
+#undef ROCKSDB_NAMESPACE
+#endif
+
+// Normal logic
 #ifndef ROCKSDB_NAMESPACE
 #define ROCKSDB_NAMESPACE rocksdb
 #endif
index 6b2f5c885ae92a153ae0eaf3e626150ca712d481..bb2de6c7b6b551b95e2ae600cb2a4251b3430dd2 100644 (file)
 
 #pragma once
 
-#include <stdint.h>
 #include <cstddef>
+#include <cstdint>
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 #if !defined(ROCKSDB_LITE) && !defined(NROCKSDB_THREAD_STATUS) && \
     defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
 #define ROCKSDB_USING_THREAD_STATUS
index f4df5c0009c0d6aa808c9420c09f748b51aa2976..9a09d8f4e714741abc0f4728fbc78b3eb4553f36 100644 (file)
@@ -5,10 +5,12 @@
 
 #pragma once
 
-#include <stdint.h>
 #include <climits>
+#include <cstdint>
 #include <vector>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 //
index c837b47f772f873e746d52c73e509d108bb8b460..57bac334682bb051975785c495581aefbdd58971 100644 (file)
@@ -5,6 +5,10 @@
 //
 #pragma once
 
+#include <string>
+
+#include "rocksdb/rocksdb_namespace.h"
+
 #ifdef FAILED
 #undef FAILED
 #endif
index 9061d45eb44fc82925faa927c05ed475ca91d4c7..7f1de8c97725c883ec719ca094e80d9aa2754e8b 100644 (file)
@@ -5,8 +5,11 @@
 
 #pragma once
 
+#include <cstddef>
 #include <unordered_map>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 // Helper methods to estimate memroy usage by std containers.
index b2a913746e84fd45ee509a3e8b62fe30d8d6b8ea..0ca899f20163ddf6315b6467ac99a863033e95c7 100644 (file)
@@ -7,6 +7,8 @@
 
 #include <cstdint>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 // Represents the types of blocks used in the block based table format.
index a7a92e3e1fcc0dd8ab183b674330461f890d3e67..1711924349a49110c845115c5bb4ac700a818b34 100644 (file)
@@ -6,7 +6,7 @@
 #pragma once
 
 #include "rocksdb/status.h"
-#include "rocksdb/iterator.h"
+#include "table/internal_iterator.h"
 
 namespace ROCKSDB_NAMESPACE {
 
index 7a57b5e987c8b1e35267fccddbf767b23aebff14..10ec08130fbbee1d1d4402985b6dad85eaba3273 100644 (file)
@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 // A list of callers for a table reader. It is used to trace the caller that
 // accesses on a block. This is only used for block cache tracing and analysis.
index d84516fba5e07a4b132a8fb721e6770bb7a48444..c7f515eeb2d2f12bce6374fa6ee852596e54e39c 100644 (file)
@@ -4,6 +4,9 @@
 //  (found in the LICENSE.Apache file in the root directory).
 
 #pragma once
+
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 // The helper function to assert the move from dynamic_cast<> to
 // static_cast<> is correct. This function is to deal with legacy code.
index 705fa2d28f3196e5b54fd69bbfde0174785251c8..19b95629771affaa536da9b0bd97798c60c1a81c 100644 (file)
@@ -10,6 +10,8 @@
 #include <queue>
 #include <utility>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 template <class T>
index 15ed6fae5e05606ab234ec6a775a13e0c9444c6b..f0b0b66d5dbc72b58680143f4c7908a46733897c 100644 (file)
@@ -7,6 +7,9 @@
 
 #pragma once
 
+#include <cstddef>
+#include <cstdint>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
index a781931733da880e3a6b916c3b39daed7b360120..33c0243e65046259db2f8322dd33192034b35d57 100644 (file)
@@ -7,6 +7,8 @@
 
 #include <functional>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 // Defers the execution of the provided function until the Defer
index 72920ca3ce914e5e060d6fadf35c95da616bee56..ce1f351db6cfe442e4b1f655dfa8d506bd09eb8b 100644 (file)
@@ -5,8 +5,7 @@
 
 #pragma once
 
-#include <cinttypes>
-
+#include "db/db_impl/db_impl.h"
 #include "util/set_comparator.h"
 
 namespace ROCKSDB_NAMESPACE {
index 871fa830eeaa2ec7ca8262434b8a1430148ed5c3..a70a980f67d0a43c5aceddd41f3c10ccf3141dcb 100644 (file)
@@ -22,6 +22,8 @@
 #include <cstdint>
 #include <type_traits>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 #ifdef TEST_UINT128_COMPAT
 #undef HAVE_UINT128_EXTENSION
 #endif
index 795efb018261b6247d639e1102e023afc507834d..dad539c4184d614daefc51ce1a8aacde445b3229 100644 (file)
@@ -13,6 +13,8 @@
 #include <cstdint>
 #include <type_traits>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 // Fast implementation of floor(log2(v)). Undefined for 0 or negative
index 9b5cfc1dc5f4184733c9ec5ccdd31ea50f393448..e0e64436ade762dcdde73117163819be503ca410 100644 (file)
@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include "rocksdb/comparator.h"
+
 namespace ROCKSDB_NAMESPACE {
 // A comparator to be used in std::set
 struct SetComparator {
index f120ca77c4f4c1d4b2c935a0bcb0301fd98d0050..94ece85d9daf31f1fd22d44d8e23031c72f4e88e 100644 (file)
@@ -21,6 +21,8 @@
 #include <mutex>
 #include <queue>
 
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 
 /// Unbounded thread-safe work queue.
index c7a9c799dc85d90fcd772e1a5bb35727636e240b..12405abe0927d104e7e81dc2098e9bcb6f01032c 100644 (file)
@@ -51,6 +51,7 @@
 #endif
 #define XXPH_NAMESPACE ROCKSDB_
 #define XXPH_INLINE_ALL
+#include <cstring>
 /* END RocksDB customizations */
 
 #if defined (__cplusplus)
index 1e6e4a25d7491428ae702125c310932558aee846..fea6b003289a64c6dc53c464676563f85a4b1979 100644 (file)
@@ -5,6 +5,10 @@
 //
 #pragma once
 
+#include <cstdint>
+
+#include "rocksdb/rocksdb_namespace.h"
+
 #ifndef ROCKSDB_LITE
 
 namespace ROCKSDB_NAMESPACE {
index 8cf756a98333102afbf5bec20c586cf9cbd4dbfa..efa73a30836060e7af378884399cb8e1a2f2ea04 100644 (file)
@@ -4,7 +4,10 @@
 //  (found in the LICENSE.Apache file in the root directory).
 
 #pragma once
-#include <cinttypes>
+
+#include <cstddef>
+#include <cstdint>
+#include <string>
 
 #include "rocksdb/rocksdb_namespace.h"
 
index cd980ade0f687a7e898bec21fb980775e02f6b1d..8f50a02dd97d13320c5fa50cbff22438e00d22c9 100644 (file)
 
 #pragma once
 
+#include <cstdint>
+#include <string>
+
+#include "rocksdb/rocksdb_namespace.h"
+
 namespace ROCKSDB_NAMESPACE {
 namespace cassandra {
 namespace {
index dc58ded21e6dea1bbffee6363165116ea20028c4..f2ca741442b71dd92ab6c12aa11b6952a6a3322b 100644 (file)
@@ -84,6 +84,10 @@ Status FileTraceReader::Read(std::string* data) {
   return s;
 }
 
+FileTraceWriter::FileTraceWriter(
+    std::unique_ptr<WritableFileWriter>&& file_writer)
+    : file_writer_(std::move(file_writer)) {}
+
 FileTraceWriter::~FileTraceWriter() { Close().PermitUncheckedError(); }
 
 Status FileTraceWriter::Close() {
index 909317fe4abc8de9ecfe8c509fa87037378abbbd..65d4831083b0e9497a44a2b586680459caa05fd1 100644 (file)
@@ -34,8 +34,7 @@ class FileTraceReader : public TraceReader {
 // FileTraceWriter allows writing RocksDB traces to a file.
 class FileTraceWriter : public TraceWriter {
  public:
-  explicit FileTraceWriter(std::unique_ptr<WritableFileWriter>&& file_writer)
-      : file_writer_(std::move(file_writer)) {}
+  explicit FileTraceWriter(std::unique_ptr<WritableFileWriter>&& file_writer);
   ~FileTraceWriter();
 
   virtual Status Write(const Slice& data) override;