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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 27, 2017, 01:13 UTC
Message-ID: <xmqqy3msfrr1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171126191510.GA1501@sigill>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> ...
> 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 was in agreement with that line of argument while the patch was discussed, and I still think the above is true.

And if the primary thing we want to address with this patch is the build breakage for those who do not have tclsh nor msgfmt, perhaps it is a more direct fix to update this part in gitk/Makefile:

    ## po-file creation rules
    XGETTEXT   ?= xgettext
    ifdef NO_MSGFMT
            MSGFMT ?= $(TCL_PATH) po/po2msg.sh
    else
            MSGFMT ?= msgfmt
            ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
                    MSGFMT := $(TCL_PATH) po/po2msg.sh
            endif
    endif

There is an identical copy-and-pasted copy of this in git-gui/Makefile, and both of them silently assume that TCL_PATH points at something usable when msgfmt is not available.

Perhaps the "else" part of the above should become a bit more careful, something along the lines of...

    else
            MSGFMT ?= msgfmt
-           ifneq ($(shell $(MSGFMT) --tcl -l C -d . /dev/null 2>/dev/null; echo $$?),0)
-                   MSGFMT := $(TCL_PATH) po/po2msg.sh
-           endif
+           MSGFMT_DRYRUN = --tcl -l C -d . /dev/null 2>/dev/null
+            ifneq ($(shell $(MSGFMT) $(MSGFMT_DRYRUN); echo $$?),0)
+               ifneq ($(shell $(TCL_PATH) po/po2msg.sh $(MSGFMT_DRYRUN); echo $$?),0)
+                    MSGFMT := $(TCL_PATH) po/po2msg.sh
+               else
+                   $(error "no usable msgfmt to build gitk; set NO_TCLTK perhaps?")
+               endif
            endif
    endif

Another reason why I think the Makefiles in gitk and git-gui projects are better targets for the fix is because they are designed to be independent projects. We should be able to do

	cd git-gui && make

in our tree, but an update to our top-level Makefile does not help those people.