On Mon, Sep 16, 2019 at 01:41:59PM +0100, Jeremy Sowden wrote: > Sebastian Priebe [0] requested Linenoise support for the CLI as an > alternative to Readline, so I thought I'd have a go at providing it. > Linenoise is a minimal, zero-config, BSD licensed, Readline replacement > used in Redis, MongoDB, and Android [1]. > > 0 - https://lore.kernel.org/netfilter-devel/4df20614cd10434b9f91080d0862dd0c@xxxxxx.group/ > 1 - https://github.com/antirez/linenoise/ > > The upstream repo doesn't contain the infrastructure for building or > installing libraries. I've taken a look at how Redis and MongoDB handle > this, and they both include the upstream sources in their trees (MongoDB > actually uses a C++ fork, Linenoise NG [2]), so I've done the same. > > 2 - https://github.com/arangodb/linenoise-ng > > Initially, I added the Linenoise header to include/ and the source to > src/, but the compiler warning flags used by upstream differ from those > used by nftables, which meant that the compiler emitted warnings when > compiling the Linenoise source and I had to edit it to fix them. Could you silent these warnings via CFLAGS just like we do with mini-gmp.{c,h}? We already cache a copy of mini-gmp.c in the tree, this would follow the same approach, just the source under src/ and the header in include/. > Since they were benign and editing the source would make it more > complicated to update from upstream in the future, I have, instead, > chosen to put everything in a separate linenoise/ directory with its > own Makefile.am and the same warning flags as upstream. > > By default, the CLI continues to be build using Readline, but passing > `with-cli=linenoise` instead causes Linenoise to be used instead. Probably good if you can also update 'nft -v' to display that nft is compiled with/without mini-gmp and also with either libreadline/linenoise. > The first two patches do a bit of tidying; the third patch adds the > Linenoise sources; the last adds Linenoise support to the CLI. No objections, please update tests/build/ to check for this new ./configure option. Thanks.
