From 1a25accc49f83771004b51766c3017118e30bb03 Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Thu, 20 Jun 2019 21:18:37 -0700 Subject: [PATCH] deps: configure and build patched libedit Teach our Makefile to orchestrate the configuration and building of our line editing library, libedit. This allows us to incorporate an upstream patch for #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. --- .gitmodules | 3 +++ Makefile | 54 +++++++++++++++++++++++++++++++++++------- build/variables.mk | 4 ++++ c-deps/libedit | 1 + c-deps/libedit-rebuild | 4 ++++ 5 files changed, 57 insertions(+), 9 deletions(-) create mode 160000 c-deps/libedit create mode 100644 c-deps/libedit-rebuild diff --git a/.gitmodules b/.gitmodules index 3e7d61ad1938..dc4ffeb3b3ec 100644 --- a/.gitmodules +++ b/.gitmodules @@ -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 diff --git a/Makefile b/Makefile index 6c495f423b49..a04656483c52 100644 --- a/Makefile +++ b/Makefile @@ -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 @@ -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. @@ -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) @@ -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 @@ -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. @@ -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 @@ -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) diff --git a/build/variables.mk b/build/variables.mk index 560fa4328a25..bf57fc961a53 100644 --- a/build/variables.mk +++ b/build/variables.mk @@ -5,6 +5,7 @@ define VALID_VARS ARCHIVE ARCHIVE_BASE ARCHIVE_EXTRAS + BASE_CGO_FLAGS_FILES BENCHES BENCHTIMEOUT BUILDCHANNEL @@ -83,6 +84,9 @@ define VALID_VARS LC_ALL LDFLAGS LIBCRYPTOPP + LIBEDIT + LIBEDIT_DIR + LIBEDIT_SRC_DIR LIBJEMALLOC LIBPROTOBUF LIBROACH diff --git a/c-deps/libedit b/c-deps/libedit new file mode 160000 index 000000000000..cec4fd710930 --- /dev/null +++ b/c-deps/libedit @@ -0,0 +1 @@ +Subproject commit cec4fd710930f96afeff418eb27941b8f91f7a84 diff --git a/c-deps/libedit-rebuild b/c-deps/libedit-rebuild new file mode 100644 index 000000000000..d5aa677ea78a --- /dev/null +++ b/c-deps/libedit-rebuild @@ -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