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

From: Jeff King <peff@peff.net>
Date: Nov 26, 2017, 19:15 UTC
Message-ID: <20171126191510.GA1501@sigill>
In-Reply-To: <CAP8UFD0d9zM9F3tLrTMiLdfoJQsOPELtmudVB6e83DiLPN5DEA@mail.gmail.com>

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

Show 13 quoted lines

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