From 2cea88dbc8c277d7403e6dd65f482fd2eb931e52 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 2 Sep 2023 15:56:37 -0400 Subject: [PATCH] Improve build sytem support for readline instead of editline Changes: - CPP variable is now `USE_READLINE` not `READLINE` - `configure.ac` supports with new CLI flag - `package.nix` supports with new configuration option - `flake.nix` CIs this (along with no markdown) Remove old Ubuntu 16.04 stop-gap too, as that is now quite old. Motivation: - editline does not build for Windows, but readline *should*. (I am still working on this in Nixpkgs at this time, however. So there will be a follow-up Nix PR removing the windows-only skipping of the readline library once I am done.) - Per https://salsa.debian.org/debian/nix/-/blob/master/debian/rules?ref_type=heads#L27 and #2551, Debian builds Nix with readline. Now we better support and CI that build configuration. This is picking up where #2551 left off, ensuring we test a few more things not merely have CPP for them. Co-authored-by: Weijia Wang <9713184+wegank@users.noreply.github.com> --- configure.ac | 29 ++++++++++++++++++----------- flake.nix | 9 +++++++++ package.nix | 12 +++++++++++- src/libcmd/repl.cc | 6 +++--- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index fdbb2629e..2594544ab 100644 --- a/configure.ac +++ b/configure.ac @@ -251,17 +251,25 @@ PKG_CHECK_MODULES([SQLITE3], [sqlite3 >= 3.6.19], [CXXFLAGS="$SQLITE3_CFLAGS $CX # Look for libcurl, a required dependency. PKG_CHECK_MODULES([LIBCURL], [libcurl], [CXXFLAGS="$LIBCURL_CFLAGS $CXXFLAGS"]) -# Look for editline, a required dependency. +# Look for editline or readline, a required dependency. # The the libeditline.pc file was added only in libeditline >= 1.15.2, # see https://github.com/troglobit/editline/commit/0a8f2ef4203c3a4a4726b9dd1336869cd0da8607, -# but e.g. Ubuntu 16.04 has an older version, so we fall back to searching for -# editline.h when the pkg-config approach fails. -PKG_CHECK_MODULES([EDITLINE], [libeditline], [CXXFLAGS="$EDITLINE_CFLAGS $CXXFLAGS"], [ - AC_CHECK_HEADERS([editline.h], [true], - [AC_MSG_ERROR([Nix requires libeditline; it was found neither via pkg-config nor its normal header.])]) - AC_SEARCH_LIBS([readline read_history], [editline], [], - [AC_MSG_ERROR([Nix requires libeditline; it was not found via pkg-config, but via its header, but required functions do not work. Maybe it is too old? >= 1.14 is required.])]) -]) +# Older versions are no longer supported. +AC_ARG_WITH( + [readline-flavor], + AS_HELP_STRING([--with-readline-flavor],[Which library to use for nice line editting with the Nix language REPL" [default=editline]]), + [readline_flavor=$withval], + [readline_flavor=editline]) +AS_CASE(["$readline_flavor"], + [editline], [ + readline_flavor_pc=libeditline + ], + [readline], [ + readline_flavor_pc=readline + AC_DEFINE([USE_READLINE], [1], [Use readline instead of editline]) + ], + [AC_MSG_ERROR([bad value "$readline_flavor" for --with-readline-flavor, must be one of: editline, readline])]) +PKG_CHECK_MODULES([EDITLINE], [$readline_flavor_pc], [CXXFLAGS="$EDITLINE_CFLAGS $CXXFLAGS"]) # Look for libsodium. PKG_CHECK_MODULES([SODIUM], [libsodium], [CXXFLAGS="$SODIUM_CFLAGS $CXXFLAGS"]) @@ -387,8 +395,7 @@ AS_CASE(["$enable_markdown"], ]) ], [no], [have_lowdown=], - [AC_MSG_ERROR([--enable-markdown must be one of: yes, no, auto])]) -AC_SUBST(HAVE_LOWDOWN, [$have_lowdown]) + [AC_MSG_ERROR([bad value "$enable_markdown" for --enable-markdown, must be one of: yes, no, auto])]) # Look for libgit2. diff --git a/flake.nix b/flake.nix index 32354a88f..c7aee7541 100644 --- a/flake.nix +++ b/flake.nix @@ -230,6 +230,15 @@ } ); + # Toggles some settings for better coverage. Windows needs these + # library combinations, and Debian build Nix with GNU readline too. + buildReadlineNoMarkdown = forAllSystems (system: + self.packages.${system}.nix.override { + enableMarkdown = false; + readlineFlavor = "readline"; + } + ); + # Perl bindings for various platforms. perlBindings = forAllSystems (system: nixpkgsFor.${system}.native.nix.perl-bindings); diff --git a/package.nix b/package.nix index 727f5e646..d0b9fc3f3 100644 --- a/package.nix +++ b/package.nix @@ -13,6 +13,7 @@ , changelog-d , curl , editline +, readline , fileset , flex , git @@ -71,6 +72,14 @@ # Whether to enable Markdown rendering in the Nix binary. , enableMarkdown ? !stdenv.hostPlatform.isWindows +# Which interactive line editor library to use for Nix's repl. +# +# Currently supported choices are: +# +# - editline (default) +# - readline +, readlineFlavor ? if stdenv.hostPlatform.isWindows then "readline" else "editline" + # Whether to compile `rl-next.md`, the release notes for the next # not-yet-released version of Nix in the manul, from the individual # change log entries in the directory. @@ -219,7 +228,7 @@ in { sqlite xz ] ++ lib.optionals (!stdenv.hostPlatform.isWindows) [ - editline + ({ inherit readline editline; }.${readlineFlavor}) ] ++ lib.optionals enableMarkdown [ lowdown ] ++ lib.optionals buildUnitTests [ @@ -279,6 +288,7 @@ in { (lib.enableFeature enableManual "doc-gen") (lib.enableFeature enableMarkdown "markdown") (lib.enableFeature installUnitTests "install-unit-tests") + (lib.withFeatureAs true "readline-flavor" readlineFlavor) ] ++ lib.optionals (!forDevShell) [ "--sysconfdir=/etc" ] ++ lib.optionals installUnitTests [ diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index dea91ba63..9c92f2b6e 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -5,7 +5,7 @@ #include -#ifdef READLINE +#ifdef USE_READLINE #include #include #else @@ -249,14 +249,14 @@ void NixRepl::mainLoop() } catch (SysError & e) { logWarning(e.info()); } -#ifndef READLINE +#ifndef USE_READLINE el_hist_size = 1000; #endif read_history(historyFile.c_str()); auto oldRepl = curRepl; curRepl = this; Finally restoreRepl([&] { curRepl = oldRepl; }); -#ifndef READLINE +#ifndef USE_READLINE rl_set_complete_func(completionCallback); rl_set_list_possib_func(listPossibleCallback); #endif