From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Christian Couder <christian.couder@gmail.com>,
Jonathan Nieder <jrnieder@gmail.com>, git <git@vger.kernel.org>,
Dominik Mahrer <teddy@teddy.ch>,
git-packagers@googlegroups.com, Todd Zullinger <tmz@pobox.com>,
Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH] Makefile: check that tcl/tk is installed
Date: Mon, 27 Nov 2017 10:13:38 +0900 [thread overview]
Message-ID: <xmqqy3msfrr1.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171126191510.GA1501@sigill> (Jeff King's message of "Sun, 26 Nov 2017 14:15:10 -0500")
Jeff King <peff@peff.net> writes:
> ...
> 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.
next prev parent reply other threads:[~2017-11-27 1:13 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-20 17:15 [PATCH] Makefile: check that tcl/tk is installed Christian Couder
2017-11-20 17:17 ` Christian Couder
2017-11-20 19:19 ` Jonathan Nieder
2017-11-20 23:58 ` Christian Couder
2017-11-26 19:15 ` Jeff King
2017-11-26 20:57 ` Christian Couder
2017-11-27 15:31 ` Jeff King
2017-11-27 1:13 ` Junio C Hamano [this message]
2017-11-27 4:31 ` Junio C Hamano
2017-11-27 4:35 ` Jeff King
2017-11-27 5:22 ` Todd Zullinger
2017-11-27 8:24 ` Christian Couder
2017-11-27 15:27 ` Jeff King
2017-11-27 23:42 ` Junio C Hamano
2017-11-28 4:35 ` Junio C Hamano
2017-11-28 14:37 ` [PATCH] travis-ci: avoid new tcl/tk build requirement Todd Zullinger
2017-11-28 15:03 ` Christian Couder
2017-11-28 16:02 ` Todd Zullinger
2017-11-28 23:47 ` Junio C Hamano
2017-11-27 9:08 ` [PATCH] Makefile: check that tcl/tk is installed Junio C Hamano
2017-11-25 20:46 ` Christian Couder
2017-11-26 3:53 ` Junio C Hamano
2017-11-26 14:00 ` Christian Couder
2017-11-26 17:43 ` Ramsay Jones
2017-11-26 18:34 ` Christian Couder
-- strict thread matches above, loose matches on Subject: below --
2017-11-15 12:52 Christian Couder
2017-11-16 1:35 ` Junio C Hamano
2017-11-17 15:35 ` Christian Couder
2017-11-17 17:42 ` Todd Zullinger
2017-11-17 22:02 ` Jeff King
2017-11-20 17:25 ` Christian Couder
2017-11-20 18:12 ` Christian Couder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy3msfrr1.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=git-packagers@googlegroups.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
--cc=teddy@teddy.ch \
--cc=tmz@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).