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
- 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
- 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:
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:
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')
.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 \
# 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
#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 {
#pragma once
+#include <cstdint>
+
#include "rocksdb/rocksdb_namespace.h"
struct CompactionIterationStats {
#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 {
#pragma once
#include "rocksdb/status.h"
+#include "rocksdb/types.h"
namespace ROCKSDB_NAMESPACE {
-class DB;
-
class PreReleaseCallback {
public:
virtual ~PreReleaseCallback() {}
#pragma once
+#include "db/dbformat.h"
#include "rocksdb/types.h"
namespace ROCKSDB_NAMESPACE {
#pragma once
#include <vector>
+#include "db/dbformat.h"
#include "rocksdb/db.h"
namespace ROCKSDB_NAMESPACE {
#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 {
// 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);
#include "rocksdb/table.h"
#include "util/gflags_compat.h"
+#include "util/random.h"
DECLARE_int32(mark_for_compaction_one_file_in);
#pragma once
+#include <functional>
#include <memory>
#include <utility>
#pragma once
+// For testing purposes
+#if ROCKSDB_NAMESPACE == 42
+#undef ROCKSDB_NAMESPACE
+#endif
+
+// Normal logic
#ifndef ROCKSDB_NAMESPACE
#define ROCKSDB_NAMESPACE rocksdb
#endif
#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
#pragma once
-#include <stdint.h>
#include <climits>
+#include <cstdint>
#include <vector>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
//
//
#pragma once
+#include <string>
+
+#include "rocksdb/rocksdb_namespace.h"
+
#ifdef FAILED
#undef FAILED
#endif
#pragma once
+#include <cstddef>
#include <unordered_map>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
// Helper methods to estimate memroy usage by std containers.
#include <cstdint>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
// Represents the types of blocks used in the block based table format.
#pragma once
#include "rocksdb/status.h"
-#include "rocksdb/iterator.h"
+#include "table/internal_iterator.h"
namespace ROCKSDB_NAMESPACE {
#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.
// (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.
#include <queue>
#include <utility>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
template <class T>
#pragma once
+#include <cstddef>
+#include <cstdint>
+
#ifdef __cplusplus
extern "C" {
#endif
#include <functional>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
// Defers the execution of the provided function until the Defer
#pragma once
-#include <cinttypes>
-
+#include "db/db_impl/db_impl.h"
#include "util/set_comparator.h"
namespace ROCKSDB_NAMESPACE {
#include <cstdint>
#include <type_traits>
+#include "rocksdb/rocksdb_namespace.h"
+
#ifdef TEST_UINT128_COMPAT
#undef HAVE_UINT128_EXTENSION
#endif
#include <cstdint>
#include <type_traits>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
// Fast implementation of floor(log2(v)). Undefined for 0 or negative
#pragma once
+#include "rocksdb/comparator.h"
+
namespace ROCKSDB_NAMESPACE {
// A comparator to be used in std::set
struct SetComparator {
#include <mutex>
#include <queue>
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
/// Unbounded thread-safe work queue.
#endif
#define XXPH_NAMESPACE ROCKSDB_
#define XXPH_INLINE_ALL
+#include <cstring>
/* END RocksDB customizations */
#if defined (__cplusplus)
//
#pragma once
+#include <cstdint>
+
+#include "rocksdb/rocksdb_namespace.h"
+
#ifndef ROCKSDB_LITE
namespace ROCKSDB_NAMESPACE {
// (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"
#pragma once
+#include <cstdint>
+#include <string>
+
+#include "rocksdb/rocksdb_namespace.h"
+
namespace ROCKSDB_NAMESPACE {
namespace cassandra {
namespace {
return s;
}
+FileTraceWriter::FileTraceWriter(
+ std::unique_ptr<WritableFileWriter>&& file_writer)
+ : file_writer_(std::move(file_writer)) {}
+
FileTraceWriter::~FileTraceWriter() { Close().PermitUncheckedError(); }
Status FileTraceWriter::Close() {
// 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;