From f422c000d0eac1bcad7ae4bad0b08c81e7600c23 Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sat, 15 Jul 2017 18:10:58 -0400 Subject: [PATCH] storage/engine: extract static library to fix windows ccl builds Since the start of our Windows support, we've been unable to build CCL binaries because ccl/storageccl/engineccl depended upon symbols from storage/engine. These symbols were not provided when engineccl was built, but rather when the final cockroach binary was linked. On Unix-like platforms, we worked around the issue by passing `-Wl,undefined` or its equivalent to ignore undefined symbols. Windows, however, had no equivalent flag (the Portable Executable format that Windows uses requires all symbols to be resolved at link time), and so we were forced to omit the broken engineccl package by building the OSS binary. This commit extracts the C and C++ code from engine and engineccl into their own static libraries, libroach and libroachccl, that are built alongside our third-party C and C++ dependencies. This means engineccl simply links against libroach *and* libroachccl so that all symbols are available at build time. (Actually, there are two Go functions in storage/engine that are called from C and so require extra magic to be available at build time; see the comments in c-deps/libroach/db.cc.) I also took the opportunity to clean up our header layout: public headers (i.e., those exposed to Go) now live in c-deps/libroach/include, while private headers live alongside their C++ source. Besides the immediate win, there are some nice side-effects: * Undefined symbols properly cause compile-time errors instead of runtime panics. `-Wl,undefined` was too large a hammer, and could mask actual undefined symbol errors. * We can directly unit test our C and C++ code. Cgo previously made this nearly impossible: *_test.go files could not use cgo, so writing tests in Go was difficult, and the C and C++ files could not be compiled outside of cgo, so writing tests in C and C++ was difficult. * The parallel-cgo patch becomes good and truly unnecessary. Fixes #14673. --- Makefile | 9 ++- build/common.mk | 24 +++++++- build/protobuf.mk | 12 +--- build/variables.mk | 5 +- c-deps/libroach-rebuild | 4 ++ c-deps/libroach/CMakeLists.txt | 55 +++++++++++++++++++ c-deps/libroach/ccl/README.md | 4 ++ .../engineccl => c-deps/libroach/ccl}/db.cc | 16 ++---- {pkg/storage/engine => c-deps/libroach}/db.cc | 46 ++++++++++------ .../db_internal.h => c-deps/libroach/db.h | 10 ++-- .../engine => c-deps/libroach}/encoding.cc | 3 +- .../engine => c-deps/libroach}/encoding.h | 0 .../libroach}/eventlistener.cc | 0 .../libroach}/eventlistener.h | 0 .../libroach/include/libroach.h | 10 +--- .../libroach/include/libroachccl.h | 12 ++-- .../protos}/cockroach/pkg/roachpb/data.pb.cc | 0 .../protos}/cockroach/pkg/roachpb/data.pb.h | 0 .../cockroach/pkg/roachpb/internal.pb.cc | 0 .../cockroach/pkg/roachpb/internal.pb.h | 0 .../cockroach/pkg/roachpb/metadata.pb.cc | 0 .../cockroach/pkg/roachpb/metadata.pb.h | 0 .../pkg/storage/engine/enginepb/mvcc.pb.cc | 0 .../pkg/storage/engine/enginepb/mvcc.pb.h | 0 .../pkg/storage/engine/enginepb/rocksdb.pb.cc | 0 .../pkg/storage/engine/enginepb/rocksdb.pb.h | 0 .../cockroach/pkg/util/hlc/timestamp.pb.cc | 0 .../cockroach/pkg/util/hlc/timestamp.pb.h | 0 .../cockroach/pkg/util/unresolved_addr.pb.cc | 0 .../cockroach/pkg/util/unresolved_addr.pb.h | 0 pkg/ccl/README.md | 6 +- pkg/ccl/storageccl/engineccl/enable_cc.cc | 5 ++ pkg/ccl/storageccl/engineccl/rocksdb.go | 17 ++---- pkg/cmd/publish-artifacts/main.go | 4 -- .../engine/cockroach_pkg_roachpb_data.pb.cc | 1 - .../cockroach_pkg_roachpb_internal.pb.cc | 1 - .../cockroach_pkg_roachpb_metadata.pb.cc | 1 - ...ach_pkg_storage_engine_enginepb_mvcc.pb.cc | 1 - ..._pkg_storage_engine_enginepb_rocksdb.pb.cc | 1 - .../cockroach_pkg_util_hlc_timestamp.pb.cc | 1 - .../cockroach_pkg_util_unresolved_addr.pb.cc | 1 - pkg/storage/engine/enable_cc.cc | 5 ++ pkg/storage/engine/rocksdb.go | 7 +-- 43 files changed, 165 insertions(+), 96 deletions(-) create mode 100644 c-deps/libroach-rebuild create mode 100644 c-deps/libroach/CMakeLists.txt create mode 100644 c-deps/libroach/ccl/README.md rename {pkg/ccl/storageccl/engineccl => c-deps/libroach/ccl}/db.cc (86%) rename {pkg/storage/engine => c-deps/libroach}/db.cc (98%) rename pkg/storage/engine/db_internal.h => c-deps/libroach/db.h (92%) rename {pkg/storage/engine => c-deps/libroach}/encoding.cc (93%) rename {pkg/storage/engine => c-deps/libroach}/encoding.h (100%) rename {pkg/storage/engine => c-deps/libroach}/eventlistener.cc (100%) rename {pkg/storage/engine => c-deps/libroach}/eventlistener.h (100%) rename pkg/storage/engine/db.h => c-deps/libroach/include/libroach.h (98%) rename pkg/ccl/storageccl/engineccl/db.h => c-deps/libroach/include/libroachccl.h (80%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/data.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/data.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/internal.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/internal.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/metadata.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/roachpb/metadata.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/util/hlc/timestamp.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/util/hlc/timestamp.pb.h (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/util/unresolved_addr.pb.cc (100%) rename {pkg/storage/engine => c-deps/libroach/protos}/cockroach/pkg/util/unresolved_addr.pb.h (100%) create mode 100644 pkg/ccl/storageccl/engineccl/enable_cc.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_roachpb_data.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_roachpb_internal.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_roachpb_metadata.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_mvcc.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_rocksdb.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_util_hlc_timestamp.pb.cc delete mode 100644 pkg/storage/engine/cockroach_pkg_util_unresolved_addr.pb.cc create mode 100644 pkg/storage/engine/enable_cc.cc diff --git a/Makefile b/Makefile index 618991eb38b9..979fbeacac47 100644 --- a/Makefile +++ b/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 diff --git a/build/common.mk b/build/common.mk index db3c7220417c..a3fa736dc7a2 100644 --- a/build/common.mk +++ b/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 diff --git a/build/protobuf.mk b/build/protobuf.mk index 4817af23e0a7..3fbc6dbbad8d 100644 --- a/build/protobuf.mk +++ b/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) diff --git a/build/variables.mk b/build/variables.mk index 3fd1c5e1e44e..6b35c279fe9a 100644 --- a/build/variables.mk +++ b/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 diff --git a/c-deps/libroach-rebuild b/c-deps/libroach-rebuild new file mode 100644 index 000000000000..4db0afa244e9 --- /dev/null +++ b/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 diff --git a/c-deps/libroach/CMakeLists.txt b/c-deps/libroach/CMakeLists.txt new file mode 100644 index 000000000000..70be7102d9d8 --- /dev/null +++ b/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 (nikhil.benesch@gmail.com) + +# 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" +) diff --git a/c-deps/libroach/ccl/README.md b/c-deps/libroach/ccl/README.md new file mode 100644 index 000000000000..c2ffe3d516b7 --- /dev/null +++ b/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). diff --git a/pkg/ccl/storageccl/engineccl/db.cc b/c-deps/libroach/ccl/db.cc similarity index 86% rename from pkg/ccl/storageccl/engineccl/db.cc rename to c-deps/libroach/ccl/db.cc index 9ff321350019..ecaa0343be0b 100644 --- a/pkg/ccl/storageccl/engineccl/db.cc +++ b/c-deps/libroach/ccl/db.cc @@ -7,16 +7,12 @@ // https://github.com/cockroachdb/cockroach/blob/master/LICENSE #include -#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 +#include +#include +#include +#include +#include "../db.h" const DBStatus kSuccess = { NULL, 0 }; diff --git a/pkg/storage/engine/db.cc b/c-deps/libroach/db.cc similarity index 98% rename from pkg/storage/engine/db.cc rename to c-deps/libroach/db.cc index b63cfb47061d..dd5d341b0e81 100644 --- a/pkg/storage/engine/db.cc +++ b/c-deps/libroach/db.cc @@ -17,28 +17,40 @@ #include #include -#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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#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__) diff --git a/pkg/storage/engine/db_internal.h b/c-deps/libroach/db.h similarity index 92% rename from pkg/storage/engine/db_internal.h rename to c-deps/libroach/db.h index a497ce1056b2..14727380d04a 100644 --- a/pkg/storage/engine/db_internal.h +++ b/c-deps/libroach/db.h @@ -14,11 +14,11 @@ // // Author: Daniel Harrison (dan@cockroachlabs.com) -#include "db.h" -#include "rocksdb/iterator.h" -#include "rocksdb/comparator.h" -#include "rocksdb/write_batch.h" -#include "rocksdb/write_batch_base.h" +#include +#include +#include +#include +#include // ToString returns a c++ string with the contents of a DBSlice. std::string ToString(DBSlice s); diff --git a/pkg/storage/engine/encoding.cc b/c-deps/libroach/encoding.cc similarity index 93% rename from pkg/storage/engine/encoding.cc rename to c-deps/libroach/encoding.cc index 2daf9f00791c..e0d117a47e03 100644 --- a/pkg/storage/engine/encoding.cc +++ b/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)] = { diff --git a/pkg/storage/engine/encoding.h b/c-deps/libroach/encoding.h similarity index 100% rename from pkg/storage/engine/encoding.h rename to c-deps/libroach/encoding.h diff --git a/pkg/storage/engine/eventlistener.cc b/c-deps/libroach/eventlistener.cc similarity index 100% rename from pkg/storage/engine/eventlistener.cc rename to c-deps/libroach/eventlistener.cc diff --git a/pkg/storage/engine/eventlistener.h b/c-deps/libroach/eventlistener.h similarity index 100% rename from pkg/storage/engine/eventlistener.h rename to c-deps/libroach/eventlistener.h diff --git a/pkg/storage/engine/db.h b/c-deps/libroach/include/libroach.h similarity index 98% rename from pkg/storage/engine/db.h rename to c-deps/libroach/include/libroach.h index a21de2f84b25..3995489e85ab 100644 --- a/pkg/storage/engine/db.h +++ b/c-deps/libroach/include/libroach.h @@ -14,8 +14,8 @@ // // Author: Peter Mattis (peter@cockroachlabs.com) -#ifndef ROACHLIB_DB_H -#define ROACHLIB_DB_H +#ifndef LIBROACH_H +#define LIBROACH_H #include #include @@ -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 diff --git a/pkg/ccl/storageccl/engineccl/db.h b/c-deps/libroach/include/libroachccl.h similarity index 80% rename from pkg/ccl/storageccl/engineccl/db.h rename to c-deps/libroach/include/libroachccl.h index 3926c3ede98c..483054e88720 100644 --- a/pkg/ccl/storageccl/engineccl/db.h +++ b/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 #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 diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/data.pb.cc b/c-deps/libroach/protos/cockroach/pkg/roachpb/data.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/data.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/roachpb/data.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/data.pb.h b/c-deps/libroach/protos/cockroach/pkg/roachpb/data.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/data.pb.h rename to c-deps/libroach/protos/cockroach/pkg/roachpb/data.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/internal.pb.cc b/c-deps/libroach/protos/cockroach/pkg/roachpb/internal.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/internal.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/roachpb/internal.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/internal.pb.h b/c-deps/libroach/protos/cockroach/pkg/roachpb/internal.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/internal.pb.h rename to c-deps/libroach/protos/cockroach/pkg/roachpb/internal.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/metadata.pb.cc b/c-deps/libroach/protos/cockroach/pkg/roachpb/metadata.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/metadata.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/roachpb/metadata.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/roachpb/metadata.pb.h b/c-deps/libroach/protos/cockroach/pkg/roachpb/metadata.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/roachpb/metadata.pb.h rename to c-deps/libroach/protos/cockroach/pkg/roachpb/metadata.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc b/c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h b/c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h rename to c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/mvcc.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc b/c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h b/c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h rename to c-deps/libroach/protos/cockroach/pkg/storage/engine/enginepb/rocksdb.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/util/hlc/timestamp.pb.cc b/c-deps/libroach/protos/cockroach/pkg/util/hlc/timestamp.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/util/hlc/timestamp.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/util/hlc/timestamp.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/util/hlc/timestamp.pb.h b/c-deps/libroach/protos/cockroach/pkg/util/hlc/timestamp.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/util/hlc/timestamp.pb.h rename to c-deps/libroach/protos/cockroach/pkg/util/hlc/timestamp.pb.h diff --git a/pkg/storage/engine/cockroach/pkg/util/unresolved_addr.pb.cc b/c-deps/libroach/protos/cockroach/pkg/util/unresolved_addr.pb.cc similarity index 100% rename from pkg/storage/engine/cockroach/pkg/util/unresolved_addr.pb.cc rename to c-deps/libroach/protos/cockroach/pkg/util/unresolved_addr.pb.cc diff --git a/pkg/storage/engine/cockroach/pkg/util/unresolved_addr.pb.h b/c-deps/libroach/protos/cockroach/pkg/util/unresolved_addr.pb.h similarity index 100% rename from pkg/storage/engine/cockroach/pkg/util/unresolved_addr.pb.h rename to c-deps/libroach/protos/cockroach/pkg/util/unresolved_addr.pb.h diff --git a/pkg/ccl/README.md b/pkg/ccl/README.md index 383e7d2f532b..19876533cba6 100644 --- a/pkg/ccl/README.md +++ b/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). diff --git a/pkg/ccl/storageccl/engineccl/enable_cc.cc b/pkg/ccl/storageccl/engineccl/enable_cc.cc new file mode 100644 index 000000000000..65169c07189e --- /dev/null +++ b/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). diff --git a/pkg/ccl/storageccl/engineccl/rocksdb.go b/pkg/ccl/storageccl/engineccl/rocksdb.go index 40a47fd7977c..c7f639f6df41 100644 --- a/pkg/ccl/storageccl/engineccl/rocksdb.go +++ b/pkg/ccl/storageccl/engineccl/rocksdb.go @@ -19,26 +19,17 @@ import ( // TODO(tamird): why does rocksdb not link jemalloc,snappy statically? -// #cgo CPPFLAGS: -I../../../../c-deps/rocksdb/include +// #cgo CPPFLAGS: -I../../../../c-deps/libroach/include +// #cgo LDFLAGS: -lroachccl +// #cgo LDFLAGS: -lroach // #cgo LDFLAGS: -lprotobuf // #cgo LDFLAGS: -lrocksdb // #cgo LDFLAGS: -lsnappy -// #cgo CXXFLAGS: -std=c++11 -Werror -Wall -Wno-sign-compare // #cgo linux LDFLAGS: -lrt -lpthread // #cgo windows LDFLAGS: -lrpcrt4 // -// // Building this package will trigger "unresolved symbol" errors -// // because it depends on C symbols defined in pkg/storage/engine, -// // which aren't linked until the final binary is built. This is the -// // platform voodoo to make the linker ignore these errors. -// // -// // TODO(tamird, #14673): make this package compile on Windows -// // (and without these flags elsewhere). -// #cgo darwin LDFLAGS: -Wl,-undefined -Wl,dynamic_lookup -// #cgo !darwin LDFLAGS: -Wl,-unresolved-symbols=ignore-all -// // #include -// #include "db.h" +// #include import "C" // VerifyBatchRepr asserts that all keys in a BatchRepr are between the specified diff --git a/pkg/cmd/publish-artifacts/main.go b/pkg/cmd/publish-artifacts/main.go index 5bdb23be9863..e8c3bf1ae180 100644 --- a/pkg/cmd/publish-artifacts/main.go +++ b/pkg/cmd/publish-artifacts/main.go @@ -214,10 +214,6 @@ func main() { { recipe := "build" - // TODO(tamird, #14673): make CCL compile on Windows. - if strings.HasSuffix(target.buildType, "windows") { - recipe = "buildoss" - } args := []string{recipe} args = append(args, fmt.Sprintf("%s=%s", "TYPE", target.buildType)) args = append(args, fmt.Sprintf("%s=%s", "GOFLAGS", extraArgs.goflags)) diff --git a/pkg/storage/engine/cockroach_pkg_roachpb_data.pb.cc b/pkg/storage/engine/cockroach_pkg_roachpb_data.pb.cc deleted file mode 100644 index 99f5b263fcfc..000000000000 --- a/pkg/storage/engine/cockroach_pkg_roachpb_data.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/roachpb/data.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_roachpb_internal.pb.cc b/pkg/storage/engine/cockroach_pkg_roachpb_internal.pb.cc deleted file mode 100644 index bdb7050c6f56..000000000000 --- a/pkg/storage/engine/cockroach_pkg_roachpb_internal.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/roachpb/internal.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_roachpb_metadata.pb.cc b/pkg/storage/engine/cockroach_pkg_roachpb_metadata.pb.cc deleted file mode 100644 index a58b1c3cd2f9..000000000000 --- a/pkg/storage/engine/cockroach_pkg_roachpb_metadata.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/roachpb/metadata.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_mvcc.pb.cc b/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_mvcc.pb.cc deleted file mode 100644 index ebf0418b8b72..000000000000 --- a/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_mvcc.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/storage/engine/enginepb/mvcc.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_rocksdb.pb.cc b/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_rocksdb.pb.cc deleted file mode 100644 index 560f636cdbca..000000000000 --- a/pkg/storage/engine/cockroach_pkg_storage_engine_enginepb_rocksdb.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/storage/engine/enginepb/rocksdb.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_util_hlc_timestamp.pb.cc b/pkg/storage/engine/cockroach_pkg_util_hlc_timestamp.pb.cc deleted file mode 100644 index c3e6ec890cac..000000000000 --- a/pkg/storage/engine/cockroach_pkg_util_hlc_timestamp.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/util/hlc/timestamp.pb.cc" diff --git a/pkg/storage/engine/cockroach_pkg_util_unresolved_addr.pb.cc b/pkg/storage/engine/cockroach_pkg_util_unresolved_addr.pb.cc deleted file mode 100644 index 634fe3f58da6..000000000000 --- a/pkg/storage/engine/cockroach_pkg_util_unresolved_addr.pb.cc +++ /dev/null @@ -1 +0,0 @@ -#include "cockroach/pkg/util/unresolved_addr.pb.cc" diff --git a/pkg/storage/engine/enable_cc.cc b/pkg/storage/engine/enable_cc.cc new file mode 100644 index 000000000000..65169c07189e --- /dev/null +++ b/pkg/storage/engine/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). diff --git a/pkg/storage/engine/rocksdb.go b/pkg/storage/engine/rocksdb.go index 1a0a0bc128e2..ab4c32fca4aa 100644 --- a/pkg/storage/engine/rocksdb.go +++ b/pkg/storage/engine/rocksdb.go @@ -52,17 +52,16 @@ import ( // TODO(tamird): why does rocksdb not link jemalloc,snappy statically? -// #cgo CPPFLAGS: -I../../../c-deps/rocksdb/include -// #cgo CPPFLAGS: -I../../../c-deps/protobuf/src +// #cgo CPPFLAGS: -I../../../c-deps/libroach/include +// #cgo LDFLAGS: -lroach // #cgo LDFLAGS: -lprotobuf // #cgo LDFLAGS: -lrocksdb // #cgo LDFLAGS: -lsnappy -// #cgo CXXFLAGS: -std=c++11 -Werror -Wall -Wno-sign-compare // #cgo linux LDFLAGS: -lrt -lpthread // #cgo windows LDFLAGS: -lrpcrt4 // // #include -// #include "db.h" +// #include import "C" var minWALSyncInterval = settings.RegisterDurationSetting(