Skip to content

Commit

Permalink
deps: configure and build patched libedit
Browse files Browse the repository at this point in the history
Teach our Makefile to orchestrate the configuration and building of our
line editing library, libedit. This allows us to incorporate an upstream
patch for cockroachdb#31843, which was causing incorrect spacing when recalling
multi-line history entries. Previously, we were attempting to link
against potentially outdated system libedits, which are likely to
exhibit the bug for years to come.

Release note (build change): a recent libedit is now bundled with
CockroachDB, which fixes some line editing bugs in the CockroachDB
console. On platforms that include libedit as part of the base system,
like macOS and FreeBSD, CockroachDB no longer links against the system
libedit.
  • Loading branch information
benesch committed Jun 21, 2019
1 parent 0205653 commit 1a25acc
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@
[submodule "c-deps/krb5"]
path = c-deps/krb5
url = https://github.com/cockroachdb/krb5.git
[submodule "c-deps/libedit"]
path = c-deps/libedit
url = https://github.com/cockroachdb/libedit.git
54 changes: 45 additions & 9 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ 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
LIBEDIT_SRC_DIR := $(C_DEPS_DIR)/libedit
LIBROACH_SRC_DIR := $(C_DEPS_DIR)/libroach
KRB5_SRC_DIR := $(C_DEPS_DIR)/krb5

Expand All @@ -436,6 +437,7 @@ 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 $(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)
KRB5_DIR := $(BUILD_DIR)/krb5$(if $(use-msan),_msan)
# Can't share with protobuf because protoc is always built for the host.
Expand All @@ -446,12 +448,13 @@ LIBJEMALLOC := $(JEMALLOC_DIR)/lib/libjemalloc.a
LIBPROTOBUF := $(PROTOBUF_DIR)/libprotobuf.a
LIBROCKSDB := $(ROCKSDB_DIR)/librocksdb.a
LIBSNAPPY := $(SNAPPY_DIR)/libsnappy.a
LIBEDIT := $(LIBEDIT_DIR)/src/.libs/libedit.a
LIBROACH := $(LIBROACH_DIR)/libroach.a
LIBROACHCCL := $(LIBROACH_DIR)/libroachccl.a
LIBKRB5 := $(KRB5_DIR)/lib/libgssapi_krb5.a
PROTOC := $(PROTOC_DIR)/protoc

C_LIBS_COMMON = $(if $(use-stdmalloc),,$(LIBJEMALLOC)) $(LIBPROTOBUF) $(LIBSNAPPY) $(LIBROCKSDB)
C_LIBS_COMMON = $(if $(use-stdmalloc),,$(LIBJEMALLOC)) $(LIBPROTOBUF) $(LIBSNAPPY) $(LIBEDIT) $(LIBROCKSDB)
C_LIBS_OSS = $(C_LIBS_COMMON) $(LIBROACH)
C_LIBS_CCL = $(C_LIBS_COMMON) $(LIBCRYPTOPP) $(LIBROACHCCL)

Expand Down Expand Up @@ -484,22 +487,39 @@ native-tag := $(subst -,_,$(TARGET_TRIPLE))$(if $(use-stdmalloc),_stdmalloc)$(if
# constraint `{native-tag}` and are built the first time a Make-driven build
# encounters a given native tag. These tags are unset when building with the Go
# toolchain directly, so these files are only compiled when building with Make.
CGO_PKGS := cli server/status storage/engine ccl/storageccl/engineccl ccl/gssapiccl
CGO_UNSUFFIXED_FLAGS_FILES := $(addprefix ./pkg/,$(addsuffix /zcgo_flags.go,$(CGO_PKGS)))
CGO_SUFFIXED_FLAGS_FILES := $(addprefix ./pkg/,$(addsuffix /zcgo_flags_$(native-tag).go,$(CGO_PKGS)))
CGO_FLAGS_FILES := $(CGO_UNSUFFIXED_FLAGS_FILES) $(CGO_SUFFIXED_FLAGS_FILES)
CGO_PKGS := \
pkg/cli \
pkg/server/status \
pkg/storage/engine \
pkg/ccl/storageccl/engineccl \
pkg/ccl/gssapiccl \
vendor/github.com/knz/go-libedit/unix
vendor/github.com/knz/go-libedit/unix-package := libedit_unix
CGO_UNSUFFIXED_FLAGS_FILES := $(addprefix ./,$(addsuffix /zcgo_flags.go,$(CGO_PKGS)))
CGO_SUFFIXED_FLAGS_FILES := $(addprefix ./,$(addsuffix /zcgo_flags_$(native-tag).go,$(CGO_PKGS)))
BASE_CGO_FLAGS_FILES := $(CGO_UNSUFFIXED_FLAGS_FILES) $(CGO_SUFFIXED_FLAGS_FILES)
CGO_FLAGS_FILES := $(BASE_CGO_FLAGS_FILES) vendor/github.com/knz/go-libedit/unix/zcgo_flags_extra.go

$(CGO_UNSUFFIXED_FLAGS_FILES): build/defs.mk.sig

$(CGO_FLAGS_FILES): Makefile
$(BASE_CGO_FLAGS_FILES): Makefile | bin/.submodules-initialized
@echo '// GENERATED FILE DO NOT EDIT' > $@
@echo >> $@
@echo '// +build $(if $(findstring $(native-tag),$@),$(native-tag),!make)' >> $@
@echo >> $@
@echo 'package $(notdir $(@D))' >> $@
@echo 'package $(if $($(@D)-package),$($(@D)-package),$(notdir $(@D)))' >> $@
@echo >> $@
@echo '// #cgo CPPFLAGS: $(addprefix -I,$(JEMALLOC_DIR)/include $(KRB_CPPFLAGS))' >> $@
@echo '// #cgo LDFLAGS: $(addprefix -L,$(CRYPTOPP_DIR) $(PROTOBUF_DIR) $(JEMALLOC_DIR)/lib $(SNAPPY_DIR) $(ROCKSDB_DIR) $(LIBROACH_DIR) $(KRB_DIR))' >> $@
@echo '// #cgo CPPFLAGS: $(addprefix -I,$(JEMALLOC_DIR)/include $(KRB_CPPFLAGS)) -DGO_LIBEDIT_NO_BUILD' >> $@
@echo '// #cgo LDFLAGS: $(addprefix -L,$(CRYPTOPP_DIR) $(PROTOBUF_DIR) $(JEMALLOC_DIR)/lib $(SNAPPY_DIR) $(LIBEDIT_DIR)/src/.libs $(ROCKSDB_DIR) $(LIBROACH_DIR) $(KRB_DIR))' >> $@
@echo 'import "C"' >> $@

vendor/github.com/knz/go-libedit/unix/zcgo_flags_extra.go: Makefile | bin/.submodules-initialized
@echo '// GENERATED FILE DO NOT EDIT' > $@
@echo >> $@
@echo 'package $($(@D)-package)' >> $@
@echo >> $@
@echo '// #cgo CPPFLAGS: -DGO_LIBEDIT_NO_BUILD' >> $@
@echo '// #cgo LDFLAGS: -ledit -lncurses' >> $@
@echo 'import "C"' >> $@

# BUILD ARTIFACT CACHING
Expand Down Expand Up @@ -602,6 +622,18 @@ $(SNAPPY_DIR)/Makefile: $(C_DEPS_DIR)/snappy-rebuild | bin/.submodules-initializ
cd $(SNAPPY_DIR) && cmake $(xcmake-flags) $(SNAPPY_SRC_DIR) \
-DCMAKE_BUILD_TYPE=Release

$(LIBEDIT_SRC_DIR)/configure.ac: | bin/.submodules-initialized

$(LIBEDIT_SRC_DIR)/configure: $(LIBEDIT_SRC_DIR)/configure.ac
cd $(LIBEDIT_SRC_DIR) && autoconf

$(LIBEDIT_DIR)/Makefile: $(C_DEPS_DIR)/libedit-rebuild $(LIBEDIT_SRC_DIR)/configure
rm -rf $(LIBEDIT_DIR)
mkdir -p $(LIBEDIT_DIR)
@# NOTE: If you change the configure flags below, bump the version in
@# $(C_DEPS_DIR)/libedit-rebuild. See above for rationale.
cd $(LIBEDIT_DIR) && $(LIBEDIT_SRC_DIR)/configure $(xconfigure-flags) --disable-examples --disable-shared

# TODO(benesch): make it possible to build libroach without CCL code. Because
# libroach and libroachccl are defined in the same CMake project, CMake requires
# that the CCL code be present even if only the OSS target will be built.
Expand Down Expand Up @@ -660,6 +692,9 @@ $(LIBPROTOBUF): $(PROTOBUF_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
$(LIBSNAPPY): $(SNAPPY_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
@uptodate $@ $(SNAPPY_SRC_DIR) || $(MAKE) --no-print-directory -C $(SNAPPY_DIR) snappy

$(LIBEDIT): $(LIBEDIT_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
@uptodate $@ $(LIBEDIT_SRC_DIR) || $(MAKE) --no-print-directory -C $(LIBEDIT_DIR)/src

$(LIBROCKSDB): $(ROCKSDB_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
@uptodate $@ $(ROCKSDB_SRC_DIR) || $(MAKE) --no-print-directory -C $(ROCKSDB_DIR) rocksdb

Expand All @@ -678,6 +713,7 @@ $(LIBKRB5): $(KRB5_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
.PHONY: protoc libcryptopp libjemalloc libprotobuf libsnappy librocksdb libroach libroachccl libkrb5
protoc: $(PROTOC)
libcryptopp: $(LIBCRYPTOPP)
libedit: $(LIBEDIT)
libjemalloc: $(LIBJEMALLOC)
libprotobuf: $(LIBPROTOBUF)
libsnappy: $(LIBSNAPPY)
Expand Down
4 changes: 4 additions & 0 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ define VALID_VARS
ARCHIVE
ARCHIVE_BASE
ARCHIVE_EXTRAS
BASE_CGO_FLAGS_FILES
BENCHES
BENCHTIMEOUT
BUILDCHANNEL
Expand Down Expand Up @@ -83,6 +84,9 @@ define VALID_VARS
LC_ALL
LDFLAGS
LIBCRYPTOPP
LIBEDIT
LIBEDIT_DIR
LIBEDIT_SRC_DIR
LIBJEMALLOC
LIBPROTOBUF
LIBROACH
Expand Down
1 change: 1 addition & 0 deletions c-deps/libedit
Submodule libedit added at cec4fd
4 changes: 4 additions & 0 deletions c-deps/libedit-rebuild
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Bump the version below when changing libedit configure flags. Search for "BUILD
ARTIFACT CACHING" in the top-level Makefile for rationale.

1

0 comments on commit 1a25acc

Please sign in to comment.