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

2017-11-27 Thread Junio C Hamano
Junio C Hamano writes: > Jeff King writes: > >> without having "wish" on the build machine. If everybody is happy with >> the BYPASS mechanism you added to address that, then I'm perfectly fine >> with it. > > OK. The topic was queued on 'pu' yesterday; lets

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

2017-11-27 Thread Junio C Hamano
Jeff King writes: > without having "wish" on the build machine. If everybody is happy with > the BYPASS mechanism you added to address that, then I'm perfectly fine > with it. OK. The topic was queued on 'pu' yesterday; lets move it forward after waiting for a few more days to

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

2017-11-27 Thread Jeff King
On Sun, Nov 26, 2017 at 09:57:14PM +0100, Christian Couder wrote: > > 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 > > > > ? > > I don't think so. I think it is just

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

2017-11-27 Thread Jeff King
On Mon, Nov 27, 2017 at 09:24:47AM +0100, Christian Couder wrote: > > 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). > > I don't think any part of my patch

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

2017-11-27 Thread Junio C Hamano
Jeff King writes: > 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. OK. >> Whether the builder wants to run the result on the same box is a >> separate and irrelevant matter. Once built and installed, a

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

2017-11-27 Thread Christian Couder
On Mon, Nov 27, 2017 at 5:35 AM, Jeff King wrote: > On Mon, Nov 27, 2017 at 01:31:13PM +0900, Junio C Hamano wrote: > >> Junio C Hamano writes: >> >> > Perhaps the "else" part of the above should become a bit more >> > careful, something along the lines of... >>

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

2017-11-26 Thread Todd Zullinger
Jeff King wrote: 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). I don't either. The general motivation there, as far as I understand, is that it's undesirable

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

2017-11-26 Thread Jeff King
On Mon, Nov 27, 2017 at 01:31:13PM +0900, Junio C Hamano wrote: > Junio C Hamano writes: > > > Perhaps the "else" part of the above should become a bit more > > careful, something along the lines of... > > > > else > > MSGFMT ?= msgfmt > > - ifneq

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

2017-11-26 Thread Junio C Hamano
Junio C Hamano 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) > -

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

2017-11-26 Thread Junio C Hamano
Jeff King 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

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

2017-11-26 Thread Christian Couder
On Sun, Nov 26, 2017 at 8:15 PM, Jeff King wrote: > 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: >> >>

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

2017-11-26 Thread Jeff King
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-344c84ddd...@teddy.ch/ > > and here are discussions related to version 1 of this patch: >

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

2017-11-26 Thread Christian Couder
On Sun, Nov 26, 2017 at 6:43 PM, Ramsay Jones wrote: > > > On 26/11/17 14:00, Christian Couder wrote: >> On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano wrote: >>> Christian Couder writes: >>> On Mon, Nov 20, 2017

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

2017-11-26 Thread Ramsay Jones
On 26/11/17 14:00, Christian Couder wrote: > On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano wrote: >> Christian Couder writes: >> >>> On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder >>> wrote: By default

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

2017-11-26 Thread Christian Couder
On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano wrote: > Christian Couder writes: > >> On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder >> wrote: >>> By default running `make install` in the root directory of the >>>

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

2017-11-25 Thread Junio C Hamano
Christian Couder writes: > On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder > wrote: >> By default running `make install` in the root directory of the >> project will set TCLTK_PATH to `wish` and then go into the "git-gui" >> and

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

2017-11-25 Thread Christian Couder
On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder wrote: > By default running `make install` in the root directory of the > project will set TCLTK_PATH to `wish` and then go into the "git-gui" > and "gitk-git" sub-directories to build and install these 2 >

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

2017-11-20 Thread Christian Couder
Hi, On Mon, Nov 20, 2017 at 8:19 PM, Jonathan Nieder wrote: > Hi, > > Christian Couder wrote: > >> By default running `make install` in the root directory of the >> project will set TCLTK_PATH to `wish` and then go into the "git-gui" >> and "gitk-git" sub-directories to build

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

2017-11-20 Thread Jonathan Nieder
Hi, Christian Couder wrote: > By default running `make install` in the root directory of the > project will set TCLTK_PATH to `wish` and then go into the "git-gui" > and "gitk-git" sub-directories to build and install these 2 > sub-projects. > > When Tcl/Tk is not installed, the above will

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

2017-11-20 Thread Christian Couder
On Fri, Nov 17, 2017 at 6:42 PM, Todd Zullinger wrote: > Christian Couder wrote: >> >> On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote: >>> >>> I suspect that this change will hurt those who package Git for other >>> people. >> >> >> Maybe a little bit,

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

2017-11-20 Thread Christian Couder
On Fri, Nov 17, 2017 at 11:02 PM, Jeff King wrote: > > I'm actually tempted to say that we should not be building the tcl parts > by default. IOW, instead of NO_TCLTK we should have USE_TCLTK. That > would also require an adjustment by package builders, but it would > hopefully be

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

2017-11-20 Thread Christian Couder
(Sorry I forgot to mark this as v2.) On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder wrote: > By default running `make install` in the root directory of the > project will set TCLTK_PATH to `wish` and then go into the "git-gui" > and "gitk-git" sub-directories to

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

2017-11-20 Thread Christian Couder
By default running `make install` in the root directory of the project will set TCLTK_PATH to `wish` and then go into the "git-gui" and "gitk-git" sub-directories to build and install these 2 sub-projects. When Tcl/Tk is not installed, the above will succeed if gettext is installed, as Tcl/Tk is

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

2017-11-17 Thread Jeff King
On Fri, Nov 17, 2017 at 12:42:58PM -0500, Todd Zullinger wrote: > > I'd rather add a separate check for msgfmt than mixing the 2 issues, > > because I think that unless it has been explicitly told to do so, Git > > should not try to build git-gui and gitk in the first place if there is > > a big

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

2017-11-17 Thread Todd Zullinger
Christian Couder wrote: On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote: I suspect that this change will hurt those who package Git for other people. Maybe a little bit, but in my opinion it should not be a big problem for them to install Tcl/Tk and its dependencies

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

2017-11-17 Thread Christian Couder
On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote: > Christian Couder writes: > >> To improve the current behavior when Tcl/Tk is not installed, >> let's just check that TCLTK_PATH points to something and error >> out right away if this is not

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

2017-11-15 Thread Junio C Hamano
Christian Couder writes: > To improve the current behavior when Tcl/Tk is not installed, > let's just check that TCLTK_PATH points to something and error > out right away if this is not the case. > > This should benefit people who actually want to install and use >

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

2017-11-15 Thread Christian Couder
By default running `make install` in the root directory of the project will set TCLTK_PATH to `wish` and then go into the "git-gui" and "gitk-git" sub-directories to build and install these 2 sub-projects. When Tcl/Tk is not installed, the above will succeed if gettext is installed, as Tcl/Tk is