Skip to content

Commit

Permalink
Revert "c-deps: fix assertion-enabled builds"
Browse files Browse the repository at this point in the history
This reverts commit 717c185.

Apparently we violate the assertions. This needs to be fixed, but until
then, let's keep the ball rolling.

One likely culprit is #38932, see:

#39034 (comment)

Release note: None
  • Loading branch information
tbg committed Aug 12, 2019
1 parent 96a27d8 commit 6d6b5c4
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 14 deletions.
7 changes: 3 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,7 @@ use-stdmalloc := $(findstring stdmalloc,$(TAGS))
use-msan := $(findstring msan,$(GOFLAGS))

# User-requested build variants.
ENABLE_LIBROACH_ASSERTIONS ?=
ENABLE_ROCKSDB_ASSERTIONS ?=
USE_ROCKSDB_ASSERTIONS :=

BUILD_DIR := $(GOPATH)/native/$(TARGET_TRIPLE)

Expand All @@ -438,10 +437,10 @@ endif
CRYPTOPP_DIR := $(BUILD_DIR)/cryptopp$(if $(use-msan),_msan)
JEMALLOC_DIR := $(BUILD_DIR)/jemalloc$(if $(use-msan),_msan)
PROTOBUF_DIR := $(BUILD_DIR)/protobuf$(if $(use-msan),_msan)
ROCKSDB_DIR := $(BUILD_DIR)/rocksdb$(if $(use-msan),_msan)$(if $(use-stdmalloc),_stdmalloc)$(if $(ENABLE_ROCKSDB_ASSERTIONS),_assert)
ROCKSDB_DIR := $(BUILD_DIR)/rocksdb$(if $(use-msan),_msan)$(if $(use-stdmalloc),_stdmalloc)$(if $(USE_ROCKSDB_ASSERTIONS),_assert)
SNAPPY_DIR := $(BUILD_DIR)/snappy$(if $(use-msan),_msan)
LIBEDIT_DIR := $(BUILD_DIR)/libedit$(if $(use-msan),_msan)
LIBROACH_DIR := $(BUILD_DIR)/libroach$(if $(use-msan),_msan)$(if $(ENABLE_LIBROACH_ASSERTIONS),_assert)
LIBROACH_DIR := $(BUILD_DIR)/libroach$(if $(use-msan),_msan)
KRB5_DIR := $(BUILD_DIR)/krb5$(if $(use-msan),_msan)
# Can't share with protobuf because protoc is always built for the host.
PROTOC_DIR := $(GOPATH)/native/$(HOST_TRIPLE)/protobuf
Expand Down
2 changes: 1 addition & 1 deletion build/teamcity-testrace.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ for pkg in $pkgspec; do
PKG="$pkg" \
TESTTIMEOUT=45m \
TESTFLAGS='-v' \
ENABLE_ROCKSDB_ASSERTIONS=1 2>&1 \
USE_ROCKSDB_ASSERTIONS=1 2>&1 \
| tee -a artifacts/testrace.log \
| go-test-teamcity
done
Expand Down
3 changes: 1 addition & 2 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ define VALID_VARS
C_LIBS_OSS
DOCGEN_TARGETS
DUPLFLAGS
ENABLE_LIBROACH_ASSERTIONS
ENABLE_ROCKSDB_ASSERTIONS
ERRORS_PATH
ERRORS_PROTO
EXECGEN_TARGETS
Expand Down Expand Up @@ -152,6 +150,7 @@ define VALID_VARS
UI_TS_CCL
UI_TS_OSS
UNAME
USE_ROCKSDB_ASSERTIONS
VERBOSE
WEBPACK
WEBPACK_DASHBOARD
Expand Down
3 changes: 2 additions & 1 deletion c-deps/libroach/ccl/key_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ rocksdb::Status DataKeyManager::LoadKeys() {
std::unique_lock<std::mutex> l(mu_);

// We should never have loaded keys before.
assert(current_key_ == nullptr);
assert(data_keys_.size() == 0);
assert(store_keys_.size() == 0);
assert(registry_ == nullptr);

std::unique_ptr<enginepbccl::DataKeysRegistry> registry(new enginepbccl::DataKeysRegistry());
Expand Down
6 changes: 0 additions & 6 deletions c-deps/libroach/options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,6 @@ class DBLogger : public rocksdb::Logger {
}
}

virtual void LogHeader(const char* format, va_list ap) override {
// RocksDB's `Logger::LogHeader()` implementation forgot to call the `Logv()` overload
// that takes severity info. Until it's fixed we can override their implementation.
Logv(rocksdb::InfoLogLevel::HEADER_LEVEL, format, ap);
}

virtual void Logv(const char* format, va_list ap) override {
// The RocksDB API tries to force us to separate the severity check (above function)
// from the actual logging (this function) by making this function pure virtual.
Expand Down

0 comments on commit 6d6b5c4

Please sign in to comment.