jonas / tig

Text-mode interface for git
https://jonas.github.io/tig/
GNU General Public License v2.0
12.46k stars 611 forks source link

cannot compile tig with readline support on macOS Big Sur (11.1) #1072

Closed edentsai closed 3 years ago

edentsai commented 3 years ago

Sorry for my poor english 🙏

I am trying to install tig with readline support on macOS to record ~/.tig_history, but the ./configure can't detect the version of installed readline library while compile tig.

In short:

$ brew info readline
readline: stable 8.1 (bottled) [keg-only]
...

$ ./autogen.sh
$ ./configure prefix="${PWD}/local" --with-readline="/usr/local/opt/readline"
...
checking readline/readline.h usability... yes
checking readline/readline.h presence... yes
checking for readline/readline.h... yes
checking readline/history.h usability... yes
checking readline/history.h presence... yes
checking for readline/history.h... yes
checking version of installed readline library... none.
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3
...

the output of ./configure displayed:

checking version of installed readline library... none
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3

I can't figure out where is the problem. :(


Details

my workstation information:

Install tig via Homebrew can't enable readline support:

$ brew uninstall tig
$ brew install tig

# tig doesn't enable readline support
$ tig --version
tig version 2.5.1
ncurses version 5.7.20081102
$ brew info tig
tig: stable 2.5.1 (bottled), HEAD
Text interface for Git repositories
https://jonas.github.io/tig/
/usr/local/Cellar/tig/2.5.1 (15 files, 880.8KB) *
  Poured from bottle on 2021-01-20 at 15:41:27
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/tig.rb
License: GPL-2.0
==> Dependencies
Required: readline ✔
==> Options
--HEAD
    Install HEAD version
==> Caveats
A sample of the default configuration has been installed to:
  /usr/local/opt/tig/share/tig/examples/tigrc
to override the system-wide default configuration, copy the sample to:
  /usr/local/etc/tigrc

Bash completion has been installed to:
  /usr/local/etc/bash_completion.d
$ brew info readline
readline: stable 8.1 (bottled) [keg-only]
Library for command-line editing
https://tiswww.case.edu/php/chet/readline/rltop.html
/usr/local/Cellar/readline/8.1 (49 files, 1.6MB)
  Poured from bottle on 2020-12-10 at 21:48:09
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/readline.rb
License: GPL-3.0-or-later
==> Caveats
readline is keg-only, which means it was not symlinked into /usr/local,
because macOS provides BSD libedit.

For compilers to find readline you may need to set:
  export LDFLAGS="-L/usr/local/opt/readline/lib"
  export CPPFLAGS="-I/usr/local/opt/readline/include"

For pkg-config to find readline you may need to set:
  export PKG_CONFIG_PATH="/usr/local/opt/readline/lib/pkgconfig"

I also tried manually compile tig from the source code, it still cannot enable readline support, I think the reason is ./configure cannot detect the version of installed readline library for me.

$ git clone git@github.com:jonas/tig.git
$ cd tig
$ git checkout tig-2.5.1

$ brew --prefix readline
/usr/local/opt/readline
$ mkdir ./local
$ ./autogen.sh
$ ./configure prefix="${PWD}/local" --with-readline="/usr/local/opt/readline" \
    && make install
...
checking readline/readline.h usability... yes
checking readline/readline.h presence... yes
checking for readline/readline.h... yes
checking readline/history.h usability... yes
checking readline/history.h presence... yes
checking for readline/history.h... yes
# ↓↓↓ NOTE: It cannot detect the version of installed readline library.
checking version of installed readline library... none
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3
...

# tig still doesn't enable readline support
$ ./local/bin/tig --version
tig version 2.5.1
ncurses version 5.7.20081102

I can't figure out where is the problem about this:

checking version of installed readline library... none

but I found a workaround to manually set ac_cv_rl_version="8.1" variable while execute ./configure:

# clean previous build/compiled
$ cd ..
$ rm -rf tig
$ git clone git@github.com:jonas/tig.git
$ cd tig
$ git checkout tig-2.5.1

$ mkdir ./local
$ ./autogen.sh
$ ac_cv_rl_version="8.1" ./configure prefix="${PWD}/local" --with-readline="/usr/local/opt/readline" \
    && make install
...
checking readline/readline.h usability... yes
checking readline/readline.h presence... yes
checking for readline/readline.h... yes
checking readline/history.h usability... yes
checking readline/history.h presence... yes
checking for readline/history.h... yes
# ↓↓↓ NOTE: It works if I manually set `ac_cv_rl_version="8.1"`
checking version of installed readline library... (cached) 8.1
...

# This time, tig enable readlline support.
$ ./local/bin/tig --version
tig version 2.5.1
ncurses version 5.7.20081102
readline version 8.1  # <<< IT WORKS

When I manually set ac_cv_rl_version="8.1" to run ./configure, it will enable readline support.

checking version of installed readline library... (cached) 8.1

but I don't know how to fix this problem without set ac_cv_rl_version variable by myself.

koutcher commented 3 years ago

Try to do export PKG_CONFIG_PATH=/usr/local/opt/readline/lib/pkgconfig before running configure.

Edit: setting PKG_CONFIG_PATH is actually useless as Tig's configure script doesn't use pkg-config for readline. Calling configure --with-readline=/usr/local/opt/readline or, as it is done by Brew, setting LDFLAGS and CPPFLAGS before calling configure is the proper way to do.

edentsai commented 3 years ago

Try to do export PKG_CONFIG_PATH=/usr/local/opt/readline/lib/pkgconfig

I tried to export LDFLAGS, CPPFLAGS and PKG_CONFIG_PATH variables but not works:

$ brew info readline
readline: stable 8.1 (bottled) [keg-only]
...
For compilers to find readline you may need to set:
  export LDFLAGS="-L/usr/local/opt/readline/lib"
  export CPPFLAGS="-I/usr/local/opt/readline/include"

For pkg-config to find readline you may need to set:
  export PKG_CONFIG_PATH="/usr/local/opt/readline/lib/pkgconfig"

$ export LDFLAGS="-L/usr/local/opt/readline/lib"
$ export CPPFLAGS="-I/usr/local/opt/readline/include"
$ export PKG_CONFIG_PATH="/usr/local/opt/readline/lib/pkgconfig"
$ env | grep -e 'LDFLAGS' -e 'CPPFLAGS' -e 'PKG_CONFIG_PATH'
LDFLAGS=-L/usr/local/opt/readline/lib
CPPFLAGS=-I/usr/local/opt/readline/include
PKG_CONFIG_PATH=/usr/local/opt/readline/lib/pkgconfig

# List the contents in the pkgconfig directory.
$ ls -A /usr/local/opt/readline/lib/pkgconfig
readline.pc

# Re-generate configure and run without `--with-readline`
$ make distclean
$ ./autogen.sh
$ ./configure prefix="${PWD}/local"
...
checking readline/readline.h usability... yes
checking readline/readline.h presence... yes
checking for readline/readline.h... yes
checking readline/history.h usability... yes
checking readline/history.h presence... yes
checking for readline/history.h... yes
checking version of installed readline library... none
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3
...

# Re-generate configure and run with `--with-readline`
$ make distclean
$ ./autogen.sh
$ ./configure prefix="${PWD}/local" --with-readline="/usr/local/opt/readline"
...
checking readline/readline.h usability... yes
checking readline/readline.h presence... yes
checking for readline/readline.h... yes
checking readline/history.h usability... yes
checking readline/history.h presence... yes
checking for readline/history.h... yes
checking version of installed readline library... none
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3
...
edentsai commented 3 years ago

I also checked the contents of installed readline library:

$ exa --all --tree --classify --group-directories-first /usr/local/opt/readline/
/usr/local/opt/readline/
├── .brew/
│  └── readline.rb
├── include/
│  └── readline/
│     ├── chardefs.h
│     ├── history.h
│     ├── keymaps.h
│     ├── readline.h
│     ├── rlconf.h
│     ├── rlstdc.h
│     ├── rltypedefs.h
│     └── tilde.h
├── lib/
│  ├── pkgconfig/
│  │  └── readline.pc
│  ├── libhistory.8.1.dylib
│  ├── libhistory.8.dylib -> libhistory.8.1.dylib
│  ├── libhistory.a
│  ├── libhistory.dylib -> libhistory.8.1.dylib
│  ├── libreadline.8.1.dylib
│  ├── libreadline.8.dylib -> libreadline.8.1.dylib
│  ├── libreadline.a
│  └── libreadline.dylib -> libreadline.8.1.dylib
├── share/
│  ├── doc/
│  │  └── readline/
│  │     ├── CHANGES
│  │     ├── INSTALL
│  │     └── README
│  ├── info/
│  │  ├── history.info
│  │  ├── readline.info
│  │  └── rluserman.info
│  ├── man/
│  │  └── man3/
│  │     ├── history.3
│  │     └── readline.3
│  └── readline/
│     ├── excallback.c
│     ├── fileman.c
│     ├── hist_erasedups.c
│     ├── hist_purgecmd.c
│     ├── histexamp.c
│     ├── manexamp.c
│     ├── rl-callbacktest.c
│     ├── rl-fgets.c
│     ├── rl.c
│     ├── rlbasic.c
│     ├── rlcat.c
│     ├── rlevent.c
│     ├── rlkeymaps.c
│     ├── rlptytest.c
│     ├── rltest.c
│     └── rlversion.c
├── CHANGELOG
├── CHANGES
├── COPYING
├── INSTALL_RECEIPT.json
├── NEWS
└── README
koutcher commented 3 years ago

Let's try something simpler. On a Mac you shouldn't need to run run configure. What if you just do: make distclean make

edentsai commented 3 years ago

It just build src/*.o and some objects, and displayed a warning about src/diff.c:

$ make distclean && make
rm -f -r doc/manual.html-chunked autom4te.cache
rm -f doc/*.toc doc/tig.1 doc/tigrc.5 doc/tigmanual.7 doc/tig.1.html doc/tigrc.5.html doc/manual.html README.html INSTALL.html NEWS.html doc/manual.html-chunked doc/manual.pdf aclocal.m4 configure
rm -f config.h config.log config.make config.status config.h.in
        CC  src/tig.o
        CC  src/types.o
        CC  src/string.o
        CC  src/util.o
        CC  src/map.o
        CC  src/argv.o
        CC  src/io.o
        CC  src/refdb.o
       GEN  src/builtin-config.c
        CC  src/builtin-config.o
        CC  src/request.o
        CC  src/line.o
        CC  src/keys.o
        CC  src/repo.o
        CC  src/options.o
        CC  src/draw.o
        CC  src/prompt.o
        CC  src/display.o
        CC  src/view.o
        CC  src/search.o
        CC  src/parse.o
        CC  src/watch.o
        CC  src/pager.o
        CC  src/log.o
        CC  src/reflog.o
        CC  src/diff.o
src/diff.c:377:27: warning: converting the enum constant to a boolean [-Wint-in-bool-context]
                   (type = LINE_DEFAULT ||
                                        ^
1 warning generated.
        CC  src/help.o
        CC  src/tree.o
        CC  src/blob.o
        CC  src/blame.o
        CC  src/refs.o
        CC  src/status.o
        CC  src/stage.o
        CC  src/main.o
        CC  src/stash.o
        CC  src/grep.o
        CC  src/ui.o
        CC  src/apps.o
        CC  src/graph.o
        CC  src/graph-v1.o
        CC  src/graph-v2.o
        CC  compat/hashtab.o
        CC  compat/utf8proc.o
      LINK  src/tig
        CC  test/tools/test-graph.o
      LINK  test/tools/test-graph
        CC  tools/doc-gen.o
      LINK  tools/doc-gen
koutcher commented 3 years ago

Are you sure it doesn't create the executable src/tig ? The warning is harmless and is corrected in master.

edentsai commented 3 years ago

I tried it again with a clean Git repository,
and just run make distclean && make && make install prefix="<PREFIX>" then the readline-support feature works:

$ git clone git@github.com:jonas/tig.git
$ cd tig
$ git checkout tig-2.5.1

$ make distclean
$ make
... (same output as the previous reply)

$ mkdir -p "${PWD}/local"
$ make install prefix="${PWD}/local"
   INSTALL  src/tig -> ~/workspace/playground/tig/local/bin
   INSTALL  tigrc -> ~/workspace/playground/tig/local/etc

$ ~/workspace/playground/tig/local/bin/tig --version
tig version 2.5.1
ncursesw version 6.2.20200212
readline version 8.1
^^^^^^^^^^^^^^^^ it finally works

but I don't know why it works. :(

I expect brew install tig should be able to enable readline support, maybe the tig of homebrew formula need to change the installation without run configure ?

Ref: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/tig.rb#L30

edentsai commented 3 years ago

Sorry for my fault 🙏

Are you sure it doesn't create the executable src/tig ?

make has created the src/tig with readline support for me.

$ make 
...
  LINK  src/tig
...

$ ./src/tig --version
tig version 2.5.1
ncursesw version 6.2.20200212
readline version 8.1
koutcher commented 3 years ago

The problem seems to be only with Catalina and Big Sur. Unfortunately I'm running an older version. Could you send a me a copy of the config.log generated after running ./configure --with-readline=/usr/local/opt/readline ?

koutcher commented 3 years ago

Does that help ?

diff --git a/tools/ax_lib_readline.m4 b/tools/ax_lib_readline.m4
index 45fb353..f667509 100644
--- a/tools/ax_lib_readline.m4
+++ b/tools/ax_lib_readline.m4
@@ -120,18 +120,18 @@ AC_CACHE_VAL(ac_cv_rl_version,

 extern int rl_gnu_readline_p;

-main()
+int main(void)
 {
    FILE *fp;
    fp = fopen("conftest.rlv", "w");
    if (fp == 0)
-       exit(1);
+       ${cf_cv_main_return:-return}(1);
    if (rl_gnu_readline_p != 1)
        fprintf(fp, "0.0\n");
    else
        fprintf(fp, "%s\n", rl_library_version ? rl_library_version : "0.0");
    fclose(fp);
-   exit(0);
+   ${cf_cv_main_return:-return}(0);
 }
 ]])],
 ac_cv_rl_version=`cat conftest.rlv`,
edentsai commented 3 years ago

The problem seems to be only with Catalina and Big Sur. Unfortunately I'm running an older version. Could you send a me a copy of the config.log generated after running ./configure --with-readline=/usr/local/opt/readline ?

I send a copy of the config.log to your email thomas.koutcher@online.fr which I found from Git commits.

edentsai commented 3 years ago

Does that help ?

diff --git a/tools/ax_lib_readline.m4 b/tools/ax_lib_readline.m4
...

I tested the changes of tools/ax_lib_readline.m4 based on the Git tag tig-v2.5.1, and it works:

👍

edentsai commented 3 years ago

I also tested few commands:

# without `--with-readline` option
$ ./autogen.sh && ./configure
...
configure: WARNING: Could not test version of installed readline library.
configure: WARNING: Minimum required version of readline is 6.3
...

$ make
...
      LINK  src/tig
...

$ ./src/tig --version
./src/tig --version
tig version 2.5.1-dirty
ncurses version 5.7.20081102

But it works with --with-readline option or use LDFLAGS and CPPFLAGS variables.

# It works with `--with-readline` option
$ ./autogen.sh && ./configure --with-readline="/usr/local/opt/readline"
...
checking version of installed readline library... 8.1
...
# use LDFLAGS and CPPFLAGS variables, without `--with-readline` option, 

$ export LDFLAGS="-L/usr/local/opt/readline/lib" 
$ export CPPFLAGS="-I/usr/local/opt/readline/include"
$ ./autogen.sh && ./configure
...
checking version of installed readline library... 8.1
...
edentsai commented 3 years ago

in another test case, it works if I just running make without running autogen.sh && ./configure,

$ make distclean
$ make
$ ./src/tig --version
tig version 2.5.1-dirty.
ncursesw version 6.2.20200212
readline version 8.1
koutcher commented 3 years ago

Thanks a lot @edentsai, config.log confirmed my intuition: Xcode 12 is less permissive than its predecessors and the code used to check readline version did not compile anymore.

make without running autogen.sh && ./configure is just fine when Homebrew is installed at the default location but we need to cover other cases as well. This is now fixed.

It will work OOTB from the next version, but if you want to install the current tig-2.5.2 with readline support from Homebrew already, you can do brew uninstall tig followed by brew install tig --HEAD (when the next version is out you'll then have to uninstall and install again to return to the bottled versions).

edentsai commented 3 years ago

Thanks a lot for your help.