Skip to content

Commit

Permalink
Merge #105282 #106187
Browse files Browse the repository at this point in the history 
105282: deps: remove go-libedit r=rickystewart a=knz

The libedit-based editor was deprecated in 23.1. We can remove it in 23.2. This simplifies the build system.

Release note: None
Fixes #105283.
Epic: CRDB-28893


106187: future: Fix a small race in the test r=miretskiy a=miretskiy

Fix a test which was racy because awaitable future may be completed immediately, while the previously arranged `WhenDone` callbacks have not finished running yet.

Fixes #99019
Fixes #99804
Fixes #102080

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>
  • Loading branch information
@knz
3 people committed Jul 5, 2023
3 parents d7c0e35 + 1d39c48 + 6880e71 commit 439a5b1
Show file tree
Hide file tree
Showing 16 changed files with 12 additions and 375 deletions.
3 changes: 0 additions & 3 deletions .gitmodules
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,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
[submodule "c-deps/geos"]
path = c-deps/geos
url = https://github.com/cockroachdb/geos.git
Expand Down
10 changes: 0 additions & 10 deletions DEPS.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5459,16 +5459,6 @@ def go_deps():
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.4.zip",
],
)
go_repository(
name = "com_github_knz_go_libedit",
build_file_proto_mode = "disable_global",
importpath = "github.com/knz/go-libedit",
sha256 = "de5a038a75f45e5c4d19321d39b85b7007b73eb77f9ec3fcca16798236fb081f",
strip_prefix = "github.com/knz/[email protected]",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/go-libedit/com_github_knz_go_libedit-v1.10.2-0.20230621133438-5f2b2e7387c5.zip",
],
)
go_repository(
name = "com_github_knz_lipgloss_convert",
build_file_proto_mode = "disable_global",
Expand Down
40 changes: 4 additions & 36 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ C_DEPS_DIR := $(abspath c-deps)
JEMALLOC_SRC_DIR := $(C_DEPS_DIR)/jemalloc
GEOS_SRC_DIR := $(C_DEPS_DIR)/geos
PROJ_SRC_DIR := $(C_DEPS_DIR)/proj
LIBEDIT_SRC_DIR := $(C_DEPS_DIR)/libedit
KRB5_SRC_DIR := $(C_DEPS_DIR)/krb5

# Derived build variants.
Expand All @@ -484,11 +483,9 @@ endif
JEMALLOC_DIR := $(BUILD_DIR)/jemalloc
GEOS_DIR := $(BUILD_DIR)/geos
PROJ_DIR := $(BUILD_DIR)/proj
LIBEDIT_DIR := $(BUILD_DIR)/libedit
KRB5_DIR := $(BUILD_DIR)/krb5

LIBJEMALLOC := $(JEMALLOC_DIR)/lib/libjemalloc.a
LIBEDIT := $(LIBEDIT_DIR)/src/.libs/libedit.a
LIBPROJ := $(PROJ_DIR)/lib/libproj$(if $(target-is-windows),_4_9).a
LIBKRB5 := $(KRB5_DIR)/lib/libgssapi_krb5.a

Expand All @@ -505,7 +502,6 @@ LIBGEOS := $(DYN_LIB_DIR)/libgeos.$(DYN_EXT)

C_LIBS_COMMON = \
$(if $(use-stdmalloc),,$(LIBJEMALLOC)) \
$(if $(target-is-windows),,$(LIBEDIT)) \
$(LIBPROJ)
C_LIBS_SHORT = $(C_LIBS_COMMON)
C_LIBS_OSS = $(C_LIBS_COMMON)
Expand Down Expand Up @@ -551,13 +547,11 @@ CGO_PKGS := \
pkg/cli/clisqlshell \
pkg/server/status \
pkg/ccl/gssapiccl \
pkg/geo/geoproj \
vendor/github.com/knz/go-libedit/unix
vendor/github.com/knz/go-libedit/unix-package := libedit_unix
pkg/geo/geoproj
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_FLAGS_FILES := $(BASE_CGO_FLAGS_FILES)

$(BASE_CGO_FLAGS_FILES): Makefile build/defs.mk.sig | bin/.submodules-initialized vendor/modules.txt
@echo "regenerating $@"
Expand All @@ -569,17 +563,7 @@ $(BASE_CGO_FLAGS_FILES): Makefile build/defs.mk.sig | bin/.submodules-initialize
@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,$(JEMALLOC_DIR)/lib $(LIBEDIT_DIR)/src/.libs $(KRB_DIR) $(PROJ_DIR)/lib)' >> $@
@echo 'import "C"' >> $@

vendor/github.com/knz/go-libedit/unix/zcgo_flags_extra.go: Makefile | bin/.submodules-initialized vendor/modules.txt
@echo "regenerating $@"
@echo '// GENERATED FILE DO NOT EDIT' > $@
@echo >> $@
@echo 'package $($(@D)-package)' >> $@
@echo >> $@
@echo '// #cgo CPPFLAGS: -DGO_LIBEDIT_NO_BUILD' >> $@
@echo '// #cgo !windows LDFLAGS: -ledit -lncurses' >> $@
@echo '// #cgo LDFLAGS: $(addprefix -L,$(JEMALLOC_DIR)/lib $(KRB_DIR) $(PROJ_DIR)/lib)' >> $@
@echo 'import "C"' >> $@

# BUILD ARTIFACT CACHING
Expand Down Expand Up @@ -650,18 +634,6 @@ $(PROJ_DIR)/Makefile: $(C_DEPS_DIR)/proj-rebuild | bin/.submodules-initialized
mkdir -p $(PROJ_DIR)
cd $(PROJ_DIR) && cmake $(xcmake-flags) $(PROJ_SRC_DIR) -DCMAKE_BUILD_TYPE=Release -DBUILD_LIBPROJ_SHARED=OFF

$(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

# Most of our C and C++ dependencies use Makefiles that are generated by CMake,
# which are rather slow, taking upwards of 500ms to determine that nothing has
# changed. The no-op case is the common case, as C and C++ code is modified
Expand Down Expand Up @@ -734,15 +706,11 @@ libgeos_inner: $(GEOS_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
$(LIBPROJ): $(PROJ_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
@uptodate $@ $(PROJ_SRC_DIR) || $(MAKE) --no-print-directory -C $(PROJ_DIR) proj

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

$(LIBKRB5): $(KRB5_DIR)/Makefile bin/uptodate .ALWAYS_REBUILD
@uptodate $@ $(KRB5_SRC_DIR)/src || $(MAKE) --no-print-directory -C $(KRB5_DIR)

# Convenient names for maintainers. Not used by other targets in the Makefile.
.PHONY: libjemalloc libgeos libproj libkrb5
libedit: $(LIBEDIT)
libjemalloc: $(LIBJEMALLOC)
libgeos: $(LIBGEOS)
libproj: $(LIBPROJ)
Expand Down Expand Up @@ -961,7 +929,7 @@ $(COCKROACHSHORT): TAGS += short
$(COCKROACHSHORT): $(C_LIBS_SHORT) | $(C_LIBS_DYNAMIC)

$(COCKROACHSQL): BUILDTARGET = ./pkg/cmd/cockroach-sql
$(COCKROACHSQL): $(if $(target-is-windows),,$(LIBEDIT))
$(COCKROACHSQL):

# For test targets, add a tag (used to enable extra assertions).
$(test-targets): TAGS += crdb_test
Expand Down
1 change: 0 additions & 1 deletion build/bazelutil/distdir_files.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,6 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/klauspost/pgzip/com_github_klauspost_pgzip-v1.2.5.zip": "1143b6417d4bb46d26dc8e6223407b84b6cd5f32e5d705cd4a9fb142220ce4ba",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/bubbline/com_github_knz_bubbline-v0.0.0-20230422210153-e176cdfe1c43.zip": "b9699be473d5dc3c1254f0e9a26f77a06cc0455135b72c2b82d85146bcfe5863",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.4.zip": "f422f7974090494e54226262586c7b34fe57b33ab7d668151ca55eba8e309c1e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/go-libedit/com_github_knz_go_libedit-v1.10.2-0.20230621133438-5f2b2e7387c5.zip": "de5a038a75f45e5c4d19321d39b85b7007b73eb77f9ec3fcca16798236fb081f",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/lipgloss-convert/com_github_knz_lipgloss_convert-v0.1.0.zip": "f9f9ffa12e7df4007cc60c87327d47ad42d1f71a80e360af4014674138de8bef",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/strtime/com_github_knz_strtime-v0.0.0-20200318182718-be999391ffa9.zip": "c1e1b06c339798387413af1444f06f31a483d4f5278ab3a91b6cd5d7cd8d91a1",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/konsorten/go-windows-terminal-sequences/com_github_konsorten_go_windows_terminal_sequences-v1.0.3.zip": "429b01413b972b108ea86bbde3d5e660913f3e8099190d07ccfb2f186bc6d837",
Expand Down
3 changes: 0 additions & 3 deletions build/variables.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ define VALID_VARS
KRB_DIR
LC_ALL
LDFLAGS
LIBEDIT
LIBEDIT_DIR
LIBEDIT_SRC_DIR
LIBGEOS
LIBJEMALLOC
LIBPROJ
Expand Down
1 change: 0 additions & 1 deletion c-deps/libedit
Submodule libedit deleted from 9dc73e
4 changes: 0 additions & 4 deletions c-deps/libedit-rebuild
View file

This file was deleted.

1 change: 0 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ require (
github.com/klauspost/compress v1.15.15
github.com/klauspost/pgzip v1.2.5
github.com/knz/bubbline v0.0.0-20230422210153-e176cdfe1c43
github.com/knz/go-libedit v1.10.2-0.20230621133438-5f2b2e7387c5
github.com/knz/strtime v0.0.0-20200318182718-be999391ffa9
github.com/kr/pretty v0.3.1
github.com/kr/text v0.2.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1524,8 +1524,6 @@ github.com/knz/bubbline v0.0.0-20230422210153-e176cdfe1c43 h1:uhwfuoL9LDMUGlxcs3
github.com/knz/bubbline v0.0.0-20230422210153-e176cdfe1c43/go.mod h1:ucXvyrucVy4jp/4afdKWNW1TVO73GMI72VNINzyT678=
github.com/knz/catwalk v0.1.4 h1:GgCxHbPp+nzyZBJcNL/CJd1aba4ACoeuI1lnsshAPkY=
github.com/knz/catwalk v0.1.4/go.mod h1:Q+Yj4ny4AXgrOOyWyDGY/HJzmbGH8MFnsUqvCAiUT5s=
github.com/knz/go-libedit v1.10.2-0.20230621133438-5f2b2e7387c5 h1:62iTw+D4JtoXL7yp4S5Ruyc82qmbWdnwuuSZ3tmcuSw=
github.com/knz/go-libedit v1.10.2-0.20230621133438-5f2b2e7387c5/go.mod h1:dmDChGdWopkB61HsdDN0/fxKAMIYljKTu+AG9uc4qVY=
github.com/knz/lipgloss-convert v0.1.0 h1:qUPUt6r8mqvi9DIV3nBPu3kEmFyHrZtXzv0BlPBPLNQ=
github.com/knz/lipgloss-convert v0.1.0/go.mod h1:S14GmtoiW/VAHqB7xEzuZOt0/G6GQ2dfjJN0fHpm30Q=
github.com/knz/strtime v0.0.0-20200318182718-be999391ffa9 h1:GQE1iatYDRrIidq4Zf/9ZzKWyrTk2sXOYc1JADbkAjQ=
Expand Down
7 changes: 0 additions & 7 deletions pkg/acceptance/generated_cli_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions pkg/cli/clisqlshell/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ go_library(
"editor_bimodal.go",
"editor_bubbline.go",
"editor_bufio.go",
"editor_editline.go",
"parser.go",
"scan_local_cmd.go",
"sql.go",
Expand Down Expand Up @@ -46,7 +45,6 @@ go_library(
"@com_github_knz_bubbline//computil",
"@com_github_knz_bubbline//editline",
"@com_github_knz_bubbline//history",
"@com_github_knz_go_libedit//:go-libedit",
],
)

Expand Down
11 changes: 1 addition & 10 deletions pkg/cli/clisqlshell/editor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@

package clisqlshell

import (
"os"

"github.com/cockroachdb/cockroach/pkg/util/envutil"
)
import "os"

// editor is the interface between the shell and a line editor.
type editor interface {
Expand Down Expand Up @@ -44,13 +40,8 @@ func getEditor(useEditor bool, displayPrompt bool) editor {
if !useEditor {
return &bufioReader{displayPrompt: displayPrompt}
}
if useLibEdit {
return &editlineReader{}
}
return &bimodalEditor{
main: &bubblineReader{},
copy: &bufioReader{displayPrompt: displayPrompt},
}
}

var useLibEdit = envutil.EnvOrDefaultBool("COCKROACH_SQL_FORCE_LIBEDIT", false)
163 changes: 0 additions & 163 deletions pkg/cli/clisqlshell/editor_editline.go
View file

This file was deleted.

Loading

0 comments on commit 439a5b1

Please sign in to comment.