Re: [PATCH] Makefile: check that tcl/tk is installed

[Jeff King <peff@peff.net>]

On Tue, Nov 21, 2017 at 12:58:17AM +0100, Christian Couder wrote:

> > Can you say more about where this comes up?
> 
> The original discussion is:
> 
> https://public-inbox.org/git/b6b12040-100f-5965-6dfd-344c84dddf96@teddy.ch/
> 
> and here are discussions related to version 1 of this patch:
> 
> https://public-inbox.org/git/20171115125200.17006-1-chriscool@tuxfamily.org/
> 
> As Peff mentions in the original discussion, at the Bloomberg Git
> sprint, we saw someone struggling to compile Git, because of these
> msgfmt and Tcl/Tk issues.

Actually, I think we had the _opposite_ problem there.

The main problem your patch fixes is that we may silently build a
version of gitk/git-gui that do not work. The "make" process completes,
but they refer to a non-existent "wish" tool, and running them will
fail.

That's potentially annoying if you wanted those tools. But if you didn't
care about them in the first place, it's fine.

The opposite problem is when you don't care about those tools, and they
_do_ break the build. And then just to get the rest of Git built, you
have to know about and set NO_TCLTK.

AFAIK that only happens if you don't have msgfmt installed. Because then
the gitk and git-gui Makefiles try to auto-fallback to implementing
msgfmt in tcl _during the build_, and there a lack of "tclsh" will break
the build.

I think your patch does say "consider setting NO_TCLTK" in that case,
which is an improvement. But it might be nicer still if it Just Worked
(either because we don't do tcl/tk by default, or because we respect
NO_GETTEXT in the gitk/git-gui Makefiles, or because our msgfmt can
fallback further to not even using tclsh).

So I'm not really against this patch, but IMHO it doesn't make the
interesting case (you don't care about tcl and are just trying to build
git for the first time) all that much better. I do also wonder if we
want to start putting these kind of run-time checks into the Makefile
itself. That's kind of what autoconf is for. As much as I hate autoconf,
is it the right advice for somebody who doesn't want to look at the
Makefile knobs to do:

  autoconf
  ./configure
  make

?

If there are deficiencies in configure.in (and I can well believe that
there are), should we be fixing it there?

-Peff