On Mon, Nov 27, 2017 at 01:31:13PM +0900, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > 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 > > Actually, at this point, I think the suggestion should primarily be > to install either msgfmt or tclsh; offering another choice to set > NO_TCLTK is OK, but it should be secondary, as the fact that the > make utility is running this recipe is a sign that the builder wants > to build gitk/git-gui. I think that's the rub, though. We hit this code path by default, so it's _not_ a sign that the builder cares about gitk. I do agree that outlining "install one of these or disable tcl" as the options is a good idea, though. > Whether the builder wants to run the result on the same box is a > separate and irrelevant matter. Once built and installed, a box > without "wish" may not be able to run the result, but installing it > after the fact will fix it without need to rebuild anything. Yeah, this side-steps the "other half" of the issue that Christian's patch addresses, which seems like the more controversial part (I don't have a strong opinion myself, though). -Peff
