Merge pull request #17035 from benesch/libroach

storage/engine: extract static library to fix windows ccl builds

Makefile

@@ -139,6 +139,9 @@ COCKROACH := ./cockroach$(SUFFIX)$(shell $(XGO) env GOEXE)
 all: $(COCKROACH)
 
 buildoss: BUILDTARGET = ./pkg/cmd/cockroach-oss
+buildoss: $(C_LIBS_OSS)
+
+$(COCKROACH) build go-install: $(C_LIBS_CCL)
 
 $(COCKROACH) build buildoss: BUILDMODE = build -i -o $(COCKROACH)
 
@@ -153,7 +156,7 @@ $(COCKROACH) build buildoss go-install: override LINKFLAGS += \
 # dependencies are rebuilt which is useful when switching between
 # normal and race test builds.
 .PHONY: build buildoss install
-$(COCKROACH) build buildoss go-install: $(C_LIBS) $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET) .buildinfo/tag .buildinfo/rev
+$(COCKROACH) build buildoss go-install: $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET) .buildinfo/tag .buildinfo/rev
 	 $(XGO) $(BUILDMODE) -v $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' $(BUILDTARGET)
 
 .PHONY: install
@@ -175,7 +178,7 @@ testbuild: gotestdashi
 	$(SHELL)
 
 .PHONY: gotestdashi
-gotestdashi: $(C_LIBS) $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET)
+gotestdashi: $(C_LIBS_CCL) $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET)
 	$(XGO) test -v $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -i $(PKG)
 
 testshort: override TESTFLAGS += -short
@@ -230,7 +233,7 @@ stressrace: TESTTIMEOUT := $(RACETIMEOUT)
 # - PKG may not contain any tests! This is handled with an `if` statement that
 # checks for the presence of a test binary before running `stress` on it.
 .PHONY: stress stressrace
-stress stressrace: $(C_LIBS) $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET)
+stress stressrace: $(C_LIBS_CCL) $(CGO_FLAGS_FILES) $(BOOTSTRAP_TARGET)
 	$(GO) list -tags '$(TAGS)' -f '$(XGO) test -v $(GOFLAGS) -tags '\''$(TAGS)'\'' -ldflags '\''$(LINKFLAGS)'\'' -i -c {{.ImportPath}} -o '\''{{.Dir}}'\''/stress.test && (cd '\''{{.Dir}}'\'' && if [ -f stress.test ]; then COCKROACH_STRESS=true stress $(STRESSFLAGS) ./stress.test -test.run '\''$(TESTS)'\'' $(if $(BENCHES),-test.bench '\''$(BENCHES)'\'') -test.timeout $(TESTTIMEOUT) $(TESTFLAGS); fi)' $(PKG) | $(SHELL)
 
 .PHONY: upload-coverage

build/common.mk

@@ -216,8 +216,9 @@ JEMALLOC_SRC_DIR := $(C_DEPS_DIR)/jemalloc
 PROTOBUF_SRC_DIR := $(C_DEPS_DIR)/protobuf
 ROCKSDB_SRC_DIR  := $(C_DEPS_DIR)/rocksdb
 SNAPPY_SRC_DIR   := $(C_DEPS_DIR)/snappy
+LIBROACH_SRC_DIR := $(C_DEPS_DIR)/libroach
 
-C_LIBS_SRCS := $(JEMALLOC_SRC_DIR) $(PROTOBUF_SRC_DIR) $(ROCKSDB_SRC_DIR) $(SNAPPY_SRC_DIR)
+C_LIBS_SRCS := $(JEMALLOC_SRC_DIR) $(PROTOBUF_SRC_DIR) $(ROCKSDB_SRC_DIR) $(SNAPPY_SRC_DIR) $(LIBROACH_SRC_DIR)
 
 HOST_TRIPLE := $(shell $$($(GO) env CC) -dumpmachine)
 
@@ -282,11 +283,13 @@ JEMALLOC_DIR := $(BUILD_DIR)/jemalloc
 PROTOBUF_DIR := $(BUILD_DIR)/protobuf
 ROCKSDB_DIR  := $(BUILD_DIR)/rocksdb$(STDMALLOC_SUFFIX)$(if $(ENABLE_ROCKSDB_ASSERTIONS),_assert)
 SNAPPY_DIR   := $(BUILD_DIR)/snappy
+LIBROACH_DIR := $(BUILD_DIR)/libroach
 # Can't share with protobuf because protoc is always built for the host.
 PROTOC_DIR := $(GOPATH)/native/$(HOST_TRIPLE)/protobuf
 PROTOC 		 := $(PROTOC_DIR)/protoc
 
-C_LIBS := $(if $(USE_STDMALLOC),,libjemalloc) libprotobuf libsnappy librocksdb
+C_LIBS_OSS = $(if $(USE_STDMALLOC),,libjemalloc) libprotobuf libsnappy librocksdb libroach
+C_LIBS_CCL = $(C_LIBS_OSS) libroachccl
 
 # Go does not permit dashes in build tags. This is undocumented. Fun!
 NATIVE_SPECIFIER_TAG := $(subst -,_,$(NATIVE_SPECIFIER))$(STDMALLOC_SUFFIX)
@@ -327,7 +330,7 @@ $(CGO_FLAGS_FILES): $(REPO_ROOT)/build/common.mk
 	@echo 'package $(notdir $(@D))' >> $@
 	@echo >> $@
 	@echo '// #cgo CPPFLAGS: -I$(JEMALLOC_DIR)/include' >> $@
-	@echo '// #cgo LDFLAGS: $(addprefix -L,$(PROTOBUF_DIR) $(JEMALLOC_DIR)/lib $(SNAPPY_DIR) $(ROCKSDB_DIR))' >> $@
+	@echo '// #cgo LDFLAGS: $(addprefix -L,$(PROTOBUF_DIR) $(JEMALLOC_DIR)/lib $(SNAPPY_DIR) $(ROCKSDB_DIR) $(LIBROACH_DIR))' >> $@
 	@echo 'import "C"' >> $@
 
 # BUILD ARTIFACT CACHING
@@ -402,6 +405,13 @@ $(SNAPPY_DIR)/Makefile: $(C_DEPS_DIR)/snappy-rebuild $(BOOTSTRAP_TARGET)
 	@# $(C_DEPS_DIR)/snappy-rebuild. See above for rationale.
 	cd $(SNAPPY_DIR) && cmake $(CMAKE_FLAGS) $(SNAPPY_SRC_DIR)
 
+$(LIBROACH_DIR)/Makefile: $(C_DEPS_DIR)/libroach-rebuild $(BOOTSTRAP_TARGET)
+	rm -rf $(LIBROACH_DIR)
+	mkdir -p $(LIBROACH_DIR)
+	@# NOTE: If you change the CMake flags below, bump the version in
+	@# $(C_DEPS_DIR)/libroach-rebuild. See above for rationale.
+	cd $(LIBROACH_DIR) && cmake $(CMAKE_FLAGS) $(LIBROACH_SRC_DIR) -DCMAKE_BUILD_TYPE=Release
+
 # We mark C and C++ dependencies as .PHONY (or .ALWAYS_REBUILD) to avoid
 # having to name the artifact (for .PHONY), which can vary by platform, and so
 # the child Makefile can determine whether the target is up to date (for both
@@ -427,6 +437,14 @@ libsnappy: $(SNAPPY_DIR)/Makefile
 librocksdb: $(ROCKSDB_DIR)/Makefile
 	@$(MAKE) --no-print-directory -C $(ROCKSDB_DIR) -j$(NCPUS) rocksdb
 
+.PHONY: libroach
+libroach: $(LIBROACH_DIR)/Makefile
+	@$(MAKE) --no-print-directory -C $(LIBROACH_DIR) -j$(NCPUS) roach
+
+.PHONY: libroachccl
+libroachccl: $(LIBROACH_DIR)/Makefile
+	@$(MAKE) --no-print-directory -C $(LIBROACH_DIR) -j$(NCPUS) roachccl
+
 .PHONY: clean-c-deps
 clean-c-deps:
 	rm -rf $(JEMALLOC_DIR) && git -C $(JEMALLOC_SRC_DIR) clean -dxf

build/protobuf.mk

@@ -19,7 +19,7 @@
 REPO_ROOT := ./cockroach
 include $(REPO_ROOT)/build/common.mk
 
-NATIVE_ROOT := $(PKG_ROOT)/storage/engine
+NATIVE_ROOT := $(LIBROACH_SRC_DIR)/protos
 GITHUB_ROOT := $(REPO_ROOT)/vendor/github.com
 
 GOGO_PROTOBUF_PACKAGE := github.com/gogo/protobuf
@@ -95,17 +95,9 @@ $(GW_SOURCES_TARGET): $(PROTOC) $(GRPC_GATEWAY_PLUGIN) $(GW_SERVER_PROTOS) $(GW_
 CPP_SOURCES_TARGET := $(LOCAL_BIN)/.cpp_protobuf_sources
 $(CPP_SOURCES_TARGET): $(PROTOC) $(CPP_PROTOS)
 	(cd $(REPO_ROOT) && git ls-files --exclude-standard --cached --others -- '*.pb.h' '*.pb.cc' | xargs rm -f)
+	mkdir -p $(NATIVE_ROOT)
 	$(PROTOC) -I.:$(GOGO_PROTOBUF_PATH):$(PROTOBUF_PATH) --cpp_out=lite:$(NATIVE_ROOT) $(CPP_PROTOS)
 	$(SED_INPLACE) -E '/gogoproto/d' $(CPP_HEADERS) $(CPP_SOURCES)
-	@# For c++, protoc generates a directory structure mirroring the package
-	@# structure (and these directories must be in the include path), but cgo
-	@# only compiles a single directory so we "symlink" the generated pb.cc files
-	@# into the storage/engine directory, taking care to avoid collisions between
-	@# files with identical names.
-	@# We use `find` and not `git ls-files` here because `git ls-files` will
-	@# include deleted files (i.e. these very "symlinks") in its output,
-	@# resulting in recursive "symlinks", which is Bad™.
-	(cd $(NATIVE_ROOT) && find . -name *.pb.cc | sed 's!./!!' | xargs -I % sh -c 'echo "#include \"%\"" > $$(echo % | tr / _)')
 	touch $@
 
 $(UI_JS): $(GO_PROTOS) $(COREOS_RAFT_PROTOS) $(YARN_INSTALLED_TARGET)

build/variables.mk

@@ -23,7 +23,8 @@ define VALID_VARS
   CONFIGURE_FLAGS
   CXX_PATH
   C_DEPS_DIR
-  C_LIBS
+  C_LIBS_CCL
+  C_LIBS_OSS
   C_LIBS_SRCS
   DUPLFLAGS
   ENABLE_ROCKSDB_ASSERTIONS
@@ -43,6 +44,8 @@ define VALID_VARS
   ISDARWIN
   JEMALLOC_DIR
   JEMALLOC_SRC_DIR
+  LIBROACH_DIR
+  LIBROACH_SRC_DIR
   LINKFLAGS
   LOCAL_BIN
   MACOS

c-deps/libroach-rebuild

@@ -0,0 +1,4 @@
+Bump the version below when changing libroach CMake flags. Search for "BUILD
+ARTIFACT CACHING" in build/common.mk for rationale.
+
+1

c-deps/libroach/CMakeLists.txt

@@ -0,0 +1,55 @@
+# Copyright 2017 The Cockroach Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# permissions and limitations under the License.
+#
+# Author: Nikhil Benesch ([email protected])
+
+# NB: Despite CMake's portability, this build configuration makes no attempt to
+# support non-GCC-like compilers.
+
+# The CXX_STANDARD property was introduced in version 3.1.
+cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
+
+project(roachlib)
+
+add_library(roach
+  db.cc
+  encoding.cc
+  eventlistener.cc
+  protos/cockroach/pkg/roachpb/data.pb.cc
+  protos/cockroach/pkg/roachpb/internal.pb.cc
+  protos/cockroach/pkg/roachpb/metadata.pb.cc
+  protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc
+  protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc
+  protos/cockroach/pkg/util/hlc/timestamp.pb.cc
+  protos/cockroach/pkg/util/unresolved_addr.pb.cc
+)
+target_include_directories(roach
+  PUBLIC  ./include
+  PRIVATE ../protobuf/src ../rocksdb/include protos
+)
+
+add_library(roachccl
+  ccl/db.cc
+)
+target_include_directories(roachccl
+  PRIVATE ../rocksdb/include
+)
+target_link_libraries(roachccl roach)
+
+set_target_properties(roach roachccl PROPERTIES
+  CXX_STANDARD 11
+  CXX_STANDARD_REQUIRED YES
+  CXX_EXTENSIONS NO
+  COMPILE_OPTIONS "-Werror;-Wall;-Wno-sign-compare"
+)

c-deps/libroach/ccl/README.md

@@ -0,0 +1,4 @@
+# Cockroach Community License (ccl) Functionality
+This tree houses our non-Apache2 licensed C++ code.
+
+Go CCL code lives in [pkg/ccl](/pkg/ccl).

pkg/ccl/storageccl/engineccl/db.cc → c-deps/libroach/ccl/db.cc

@@ -7,16 +7,12 @@
 //     https://github.com/cockroachdb/cockroach/blob/master/LICENSE
 
 #include <memory>
-#include "rocksdb/iterator.h"
-#include "rocksdb/comparator.h"
-#include "rocksdb/write_batch.h"
-#include "rocksdb/utilities/write_batch_with_index.h"
-#include "../../../storage/engine/db_internal.h"
-#include "db.h"
-
-extern "C" {
-#include "_cgo_export.h"
-}  // extern "C"
+#include <rocksdb/iterator.h>
+#include <rocksdb/comparator.h>
+#include <rocksdb/write_batch.h>
+#include <rocksdb/utilities/write_batch_with_index.h>
+#include <libroachccl.h>
+#include "../db.h"
 
 const DBStatus kSuccess = { NULL, 0 };
 

pkg/storage/engine/db.cc → c-deps/libroach/db.cc

@@ -17,28 +17,40 @@
 
 #include <algorithm>
 #include <google/protobuf/stubs/stringprintf.h>
-#include "rocksdb/cache.h"
-#include "rocksdb/db.h"
-#include "rocksdb/env.h"
-#include "rocksdb/filter_policy.h"
-#include "rocksdb/ldb_tool.h"
-#include "rocksdb/merge_operator.h"
-#include "rocksdb/options.h"
-#include "rocksdb/slice_transform.h"
-#include "rocksdb/statistics.h"
-#include "rocksdb/sst_file_writer.h"
-#include "rocksdb/table.h"
-#include "rocksdb/utilities/write_batch_with_index.h"
-#include "cockroach/pkg/roachpb/data.pb.h"
-#include "cockroach/pkg/roachpb/internal.pb.h"
-#include "cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h"
-#include "cockroach/pkg/storage/engine/enginepb/mvcc.pb.h"
+#include <rocksdb/cache.h>
+#include <rocksdb/db.h>
+#include <rocksdb/env.h>
+#include <rocksdb/filter_policy.h>
+#include <rocksdb/ldb_tool.h>
+#include <rocksdb/merge_operator.h>
+#include <rocksdb/options.h>
+#include <rocksdb/slice_transform.h>
+#include <rocksdb/statistics.h>
+#include <rocksdb/sst_file_writer.h>
+#include <rocksdb/table.h>
+#include <rocksdb/utilities/write_batch_with_index.h>
+#include "protos/cockroach/pkg/roachpb/data.pb.h"
+#include "protos/cockroach/pkg/roachpb/internal.pb.h"
+#include "protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h"
+#include "protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h"
 #include "db.h"
 #include "encoding.h"
 #include "eventlistener.h"
 
 extern "C" {
-#include "_cgo_export.h"
+static void __attribute__((noreturn)) die_missing_symbol(const char* name) {
+  fprintf(stderr, "%s symbol missing; expected to be supplied by Go\n", name);
+  abort();
+}
+
+// These are Go functions exported by storage/engine. We provide these stubs,
+// which simply panic if called, to to allow intermediate build products to link
+// successfully. Otherwise, when building ccl/storageccl/engineccl, Go will
+// complain that these symbols are undefined. Because these stubs are marked
+// "weak", they will be replaced by their proper implementation in
+// storage/engine when the final cockroach binary is linked.
+void __attribute__((weak)) rocksDBLog(char*, int) { die_missing_symbol(__func__); }
+char* __attribute__((weak)) prettyPrintKey(DBKey) { die_missing_symbol(__func__); }
 }  // extern "C"
 
 #if defined(COMPILER_GCC) || defined(__clang__)

pkg/storage/engine/db_internal.h → c-deps/libroach/db.h

@@ -14,11 +14,11 @@
 //
 // Author: Daniel Harrison ([email protected])
 
-#include "db.h"
-#include "rocksdb/iterator.h"
-#include "rocksdb/comparator.h"
-#include "rocksdb/write_batch.h"
-#include "rocksdb/write_batch_base.h"
+#include <rocksdb/iterator.h>
+#include <rocksdb/comparator.h>
+#include <rocksdb/write_batch.h>
+#include <rocksdb/write_batch_base.h>
+#include <libroach.h>
 
 // ToString returns a c++ string with the contents of a DBSlice.
 std::string ToString(DBSlice s);

pkg/storage/engine/encoding.cc → c-deps/libroach/encoding.cc

@@ -18,8 +18,7 @@
 #include "rocksdb/slice.h"
 #include "encoding.h"
 
-// TODO(pmattis): These functions are not tested. Doing so is made
-// difficult by "go test" because _test.go files cannot 'import "C"'.
+// TODO(benesch): Set up a CI pipeline to test these functions.
 
 void EncodeUint32(std::string* buf, uint32_t v) {
   const uint8_t tmp[sizeof(v)] = {

pkg/storage/engine/db.h → c-deps/libroach/include/libroach.h

@@ -14,8 +14,8 @@
 //
 // Author: Peter Mattis ([email protected])
 
-#ifndef ROACHLIB_DB_H
-#define ROACHLIB_DB_H
+#ifndef LIBROACH_H
+#define LIBROACH_H
 
 #include <stdbool.h>
 #include <stddef.h>
@@ -295,8 +295,4 @@ DBStatus DBEnvWriteFile(DBEngine* db, DBSlice path, DBSlice contents);
 }  // extern "C"
 #endif
 
-#endif // ROACHLIB_DB_H
-
-// local variables:
-// mode: c++
-// end:
+#endif // LIBROACH_H

pkg/ccl/storageccl/engineccl/db.h → c-deps/libroach/include/libroachccl.h

@@ -6,10 +6,10 @@
 //
 //     https://github.com/cockroachdb/cockroach/blob/master/LICENSE
 
-#ifndef ROACHLIBCCL_DB_H
-#define ROACHLIBCCL_DB_H
+#ifndef LIBROACHCCL_H
+#define LIBROACHCCL_H
 
-#include "../../../storage/engine/db.h"
+#include <libroach.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -24,8 +24,4 @@ DBStatus DBBatchReprVerify(
 }  // extern "C"
 #endif
 
-#endif // ROACHLIBCCL_DB_H
-
-// local variables:
-// mode: c++
-// end:
+#endif // LIBROACHCCL_H

pkg/ccl/README.md

@@ -1,7 +1,9 @@
 # Cockroach Community License (ccl) Functionality
-This tree is intended to house the bulk of our non-Apache2 licensed packages.
-By convention, all packages under this tree have the suffix `ccl`.
+This tree is intended to house our non-Apache2 licensed Go packages. By
+convention, all packages under this tree have the suffix `ccl`.
 
 Grouping CCL packages into one tree and clearly labeling all CCL packages
 with a recognizable suffix will hopefully make it easier to identify and prevent
 introducing any accidental dependencies on ccl from Apache2 packages.
+
+C++ CCL code lives in [c-deps/libroach/ccl](/c-deps/libroach/ccl).

pkg/ccl/storageccl/engineccl/enable_cc.cc

@@ -0,0 +1,5 @@
+// Since we link against C++ libraries, like RocksDB and libroach, we need to
+// link against the C++ standard library. This presence of this file convinces
+// cgo to link this package using the C++ compiler instead of the C compiler,
+// which brings in the appropriate, platform-specific C++ library (e.g., libc++
+// on macOS or libstdc++ on Linux).