{"thread":{"id":"47212","subject":"[PATCH] Makefile: check that tcl/tk is installed","startedAt":"2017-11-15T12:52:18Z","lastAt":"2017-11-20T18:12:40Z","messageCount":7,"participants":["Christian Couder","Junio C Hamano","Todd Zullinger","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"332603","messageId":"20171115125200.17006-1-chriscool@tuxfamily.org","threadId":"47212","inReplyTo":null,"subject":"[PATCH] Makefile: check that tcl/tk is installed","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-11-15T12:52:00Z","receivedAt":"2017-11-15T12:52:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"By default running `make install` in the root directory of the\nproject will set TCLTK_PATH to `wish` and then go into the \"git-gui\"\nand \"gitk-git\" sub-directories to build and install these 2\nsub-projects.\n\nWhen Tcl/Tk is not installed, the above will succeed if gettext\nis installed, as Tcl/Tk is only required as a substitute for msgfmt\nwhen msgfmt is not installed. But then running the installed gitk\nand git-gui will fail.\n\nIf neither Tcl/Tk nor gettext are installed, then processing po\nfiles will fail in the git-gui directory. The error message when\nthis happens is very confusing to new comers as it is difficult\nto understand that we tried to use Tcl/Tk as a substitute for\nmsgfmt, and that the solution is to either install gettext or\nTcl/Tk, or to set both NO_GETTEXT and NO_TCLTK.\n\nTo improve the current behavior when Tcl/Tk is not installed,\nlet's just check that TCLTK_PATH points to something and error\nout right away if this is not the case.\n\nThis should benefit people who actually want to install and use\ngit-gui or gitk as they will have to install Tcl/Tk anyway, and\nit is better for them if they are told about installing it soon\nin the build process, rather than if they have to debug it after\ninstalling.\n\nFor people who don't want to use git-gui or gitk, this forces\nthem to specify NO_TCLTK. If they don't have gettext, this will\nmake it much easier to fix the problems they would have had to\nfix anyway. If they have gettext, setting NO_TCLTK is a small\nadditional step they will have to make, but it might be a good\nthing as it will not install what they don't want and it will\nbuild a bit faster.\n\nSigned-off-by: Christian Couder \n---\n Makefile | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex ee9d5eb11e..ada6164e15 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1636,6 +1636,13 @@ ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\n \n+ifndef NO_TCLTK\n+\thas_tcltk := $(shell type $(TCLTK_PATH) 2>/dev/null)\n+\tifndef has_tcltk\n+$(error \"Tcl/Tk is not installed ('$(TCLTK_PATH)' not found). Consider setting NO_TCLTK or installing Tcl/Tk\")\n+\tendif\n+endif\n+\n ifeq ($(PERL_PATH),)\n NO_PERL = NoThanks\n endif\n-- \n2.15.0.165.g42c5887\n\n"},{"id":"332635","messageId":"xmqqbmk3xaxg.fsf@gitster.mtv.corp.google.com","threadId":"47212","inReplyTo":"20171115125200.17006-1-chriscool@tuxfamily.org","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-16T01:35:39Z","receivedAt":"2017-11-16T01:35:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> To improve the current behavior when Tcl/Tk is not installed,\n> let's just check that TCLTK_PATH points to something and error\n> out right away if this is not the case.\n>\n> This should benefit people who actually want to install and use\n> git-gui or gitk as they will have to install Tcl/Tk anyway, and\n> it is better for them if they are told about installing it soon\n> in the build process, rather than if they have to debug it after\n> installing.\n\nNot objecting, but thinking aloud if this change makes sense.\n\nIf you are building Git for your own use on the same box, which is\npresumably the majority of \"build failed and I have no clue how to\nfix\" case that needs help, if you want gui tools, you need to have\ntcl/tk installed anyway, whether you have msgfmt installed. This\nseems to be the _only_ class of users this patch wants to cater to.\n\nI wonder if we are hurting people who are not in that category.\n\n - To run gui tools, tcl/tk must be available at runtime, but tcl/tk\n is not necessary in the packager's environment to produce a\n package of Git that contains working git-gui and gitk that will\n be used on an end-user box with tcl/tk installed, as long as the\n packager's environment has a working msgfmt.\n\n - To process .po files for the gui tools in the packager's\n environment without msgfmt, tcl/tk is required.\n\nI suspect that this change will hurt those who package Git for other\npeople.\n\nIt used to be that, as long as they have msgfmt installed, they only\nneeded to _know_ what the path on the users' box to \"wish\" is, and\nset it to TCLTK_PATH, and if they are distro packagers, most likely\nthey already have such an automated set-up working. Now with this\nchange, they are forced to install tcl/tk on their possibly headless\nbox where tcl/tk is useless, and worse yet, an attempt to install it\nmay bring in tons of unwanted stuff related to X that is irrelevant\non such a headless development environment.\n\nI doubt that this is quite a good trade-off; it feels that this\nburdens packagers a bit too much, and we may need a way to override\nthis new check further. I think \"If I cannot run either wish or\nmsgfmt, then barf and give an error message\" might at least be\nneeded. Am I misinterpreting the motivation of the patch?\n\n> diff --git a/Makefile b/Makefile\n> index ee9d5eb11e..ada6164e15 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1636,6 +1636,13 @@ ifeq ($(TCLTK_PATH),)\n> NO_TCLTK = NoThanks\n> endif\n> \n> +ifndef NO_TCLTK\n> +\thas_tcltk := $(shell type $(TCLTK_PATH) 2>/dev/null)\n> +\tifndef has_tcltk\n> +$(error \"Tcl/Tk is not installed ('$(TCLTK_PATH)' not found). Consider setting NO_TCLTK or installing Tcl/Tk\")\n> +\tendif\n> +endif\n> +\n> ifeq ($(PERL_PATH),)\n> NO_PERL = NoThanks\n> endif\n"},{"id":"332758","messageId":"CAP8UFD1hcFLMvNsXONPNbxZhTbHVzSMdRgCB9m=ZGeSTpMsYew@mail.gmail.com","threadId":"47212","inReplyTo":"xmqqbmk3xaxg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-11-17T15:35:23Z","receivedAt":"2017-11-17T15:35:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> To improve the current behavior when Tcl/Tk is not installed,\n>> let's just check that TCLTK_PATH points to something and error\n>> out right away if this is not the case.\n>>\n>> This should benefit people who actually want to install and use\n>> git-gui or gitk as they will have to install Tcl/Tk anyway, and\n>> it is better for them if they are told about installing it soon\n>> in the build process, rather than if they have to debug it after\n>> installing.\n>\n> Not objecting, but thinking aloud if this change makes sense.\n>\n> If you are building Git for your own use on the same box, which is\n> presumably the majority of \"build failed and I have no clue how to\n> fix\" case that needs help, if you want gui tools, you need to have\n> tcl/tk installed anyway, whether you have msgfmt installed. This\n> seems to be the _only_ class of users this patch wants to cater to.\n>\n> I wonder if we are hurting people who are not in that category.\n>\n> - To run gui tools, tcl/tk must be available at runtime, but tcl/tk\n> is not necessary in the packager's environment to produce a\n> package of Git that contains working git-gui and gitk that will\n> be used on an end-user box with tcl/tk installed, as long as the\n> packager's environment has a working msgfmt.\n>\n> - To process .po files for the gui tools in the packager's\n> environment without msgfmt, tcl/tk is required.\n>\n> I suspect that this change will hurt those who package Git for other\n> people.\n\nMaybe a little bit, but in my opinion it should not be a big problem\nfor them to install Tcl/Tk and its dependencies on the build machine.\n\n> It used to be that, as long as they have msgfmt installed, they only\n> needed to _know_ what the path on the users' box to \"wish\" is, and\n> set it to TCLTK_PATH, and if they are distro packagers, most likely\n> they already have such an automated set-up working. Now with this\n> change, they are forced to install tcl/tk on their possibly headless\n> box where tcl/tk is useless, and worse yet, an attempt to install it\n> may bring in tons of unwanted stuff related to X that is irrelevant\n> on such a headless development environment.\n\nYeah, but if they build gitk and git-gui, there is a significant\nchance that they build other graphical software too, and that this\nwill require installing stuff related to X anyway.\n\n> I doubt that this is quite a good trade-off; it feels that this\n> burdens packagers a bit too much, and we may need a way to override\n> this new check further.\n\nI am ok to let packagers override this new check. For example they\ncould set a flag like BYPASS_TCLTK_CHECK and the new check would be:\n\nifndef NO_TCLTK\n ifndef BYPASS_TCLTK_CHECK\n has_tcltk := $(shell type $(TCLTK_PATH) 2>/dev/null)\n ifndef has_tcltk\n$(error \"Tcl/Tk is not installed ('$(TCLTK_PATH)' not found). Consider\nsetting NO_TCLTK or installing Tcl/Tk\")\n endif\n endif\nendif\n\nOf course BYPASS_TCLTK_CHECK would have to be documented at the same\nplace where NO_TCLTK and TCLTK_PATH are already documented.\n\nIn general I think packagers are much more able to deal with those\nkinds of problems than most regular developers who want to hack on\nGit.\nSo asking packagers to either set NO_TCLTK or BYPASS_TCLTK_CHECK or to\ninstall Tcl/Tk would not burden them much, especially compared to what\nregular developers have to deal with these days when trying to build\nGit.\n\n> I think \"If I cannot run either wish or\n> msgfmt, then barf and give an error message\" might at least be\n> needed. Am I misinterpreting the motivation of the patch?\n\nI'd rather add a separate check for msgfmt than mixing the 2 issues,\nbecause I think that unless it has been explicitly told to do so, Git\nshould not try to build git-gui and gitk in the first place if there\nis a big chance that those tools will not work.\n"},{"id":"332764","messageId":"20171117174258.GP3693@zaya.teonanacatl.net","threadId":"47212","inReplyTo":"CAP8UFD1hcFLMvNsXONPNbxZhTbHVzSMdRgCB9m=ZGeSTpMsYew@mail.gmail.com","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2017-11-17T17:42:58Z","receivedAt":"2017-11-17T17:43:10Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Christian Couder wrote:\n> On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote:\n>> I suspect that this change will hurt those who package Git for other \n>> people.\n>\n> Maybe a little bit, but in my opinion it should not be a big problem \n> for them to install Tcl/Tk and its dependencies on the build machine.\n\nIt's not a big burden, but it is a seemingly unnecessary build-time \ndependency.\n\n>> It used to be that, as long as they have msgfmt installed, they only \n>> needed to _know_ what the path on the users' box to \"wish\" is, and \n>> set it to TCLTK_PATH, and if they are distro packagers, most likely \n>> they already have such an automated set-up working. Now with this \n>> change, they are forced to install tcl/tk on their possibly headless \n>> box where tcl/tk is useless, and worse yet, an attempt to install it \n>> may bring in tons of unwanted stuff related to X that is irrelevant \n>> on such a headless development environment.\n>\n> Yeah, but if they build gitk and git-gui, there is a significant \n> chance that they build other graphical software too, and that this \n> will require installing stuff related to X anyway.\n\nMost distributions build packages in individual container or chroots, \nto increase the stability and reproducibility of the builds. So \npackage builds don't run on systems where any deps have already been \ninstalled.\n\nTo be fair, it looks like pulling in tcl/tk would add only around 8MB \nto the Fedora build root for git. That's not egregious, to be sure. \nBut it really isn't a necessary build-time dependency either. I don't \nknow if there are other distros who would strongly object to pulling \nin tcl/tk. Some are much more sensitive to build root sizes and \nunnecessary dependencies.\n\n> In general I think packagers are much more able to deal with those \n> kinds of problems than most regular developers who want to hack on \n> Git.\n\nI agree. Packagers also provide git builds to the vast majority of \nend-users, so we should make their task easier whenever possible. :)\n\n> So asking packagers to either set NO_TCLTK or BYPASS_TCLTK_CHECK or to \n> install Tcl/Tk would not burden them much, especially compared to what \n> regular developers have to deal with these days when trying to build \n> Git.\n\nPresuming this new BYPASS_TCLTK_CHECK is communicated well and that \nthe failure when not using it is clear, this doesn't seem likely to \ncause problems. (I'll leave it to others whether there's a better way \nto solve the msgfmt fallthrough issue. I didn't even know such a \nfallthrough existed until yesterday.)\n\nI think it's important to ensure that automated package builds of a \nnewer git don't simply skip parts of the build which used to work and \nso packagers reading the failed builds logs can easily see what they \nneed to adjust.\n\nJust dropping the new variable in the Makefile and waiting for package \nbuilds to fail or not package gitk & git-gui at the next release would \nbe a bit unkind, I think. Posting this to the git-packagers group[1] \nwhich Ævar created would be useful. It /might/ even be worth asking \nthere if any distros have strong opinions on the subject.\n\n[1] https://groups.google.com/forum/#!forum/git-packagers and\n git-packagers@googlegroups.com\n\n>> I think \"If I cannot run either wish or msgfmt, then barf and give \n>> an error message\" might at least be needed. Am I misinterpreting \n>> the motivation of the patch?\n>\n> I'd rather add a separate check for msgfmt than mixing the 2 issues, \n> because I think that unless it has been explicitly told to do so, \n> Git should not try to build git-gui and gitk in the first place if \n> there is a big chance that those tools will not work.\n\nIf that's a motivation, wouldn't a check in the gitk and git-gui \nscripts handle it? That would provide an error at run time to the \nuser. This change is about helping the user who builds their own git \nand then runs it, so if they built git without wish installed and then \nran git-gui, they'd get a clear error that wish is missing and could \neasily install it. It's not needed for the build, so they wouldn't \nneed to rebuild anything.\n\nSomething like this:\n\ndiff --git i/gitk-git/gitk w/gitk-git/gitk\nindex a14d7a16b2..f9f28a164a 100755\n--- i/gitk-git/gitk\n+++ w/gitk-git/gitk\n@@ -1,5 +1,6 @@\n #!/bin/sh\n-# Tcl ignores the next line -*- tcl -*- \\\n+# Tcl ignores the next 2 lines -*- tcl -*- \\\n+type wish >/dev/null 2>&1 || { echo \"error: wish not found\" >&2; exit 1; }; \\\n exec wish \"$0\" -- \"$@\"\n \n # Copyright © 2005-2016 Paul Mackerras. All rights reserved.\n\nmaybe? (The error message is certainly open for improvement.)\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nA fool's brain digests philosophy into folly, science into\nsuperstition, and art into pedantry. Hence University education.\n -- George Bernard Shaw\n\n"},{"id":"332775","messageId":"20171117220210.6xqi26mabbyvxc2m@sigill.intra.peff.net","threadId":"47212","inReplyTo":"20171117174258.GP3693@zaya.teonanacatl.net","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-17T22:02:10Z","receivedAt":"2017-11-17T22:02:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 17, 2017 at 12:42:58PM -0500, Todd Zullinger wrote:\n\n> > I'd rather add a separate check for msgfmt than mixing the 2 issues,\n> > because I think that unless it has been explicitly told to do so, Git\n> > should not try to build git-gui and gitk in the first place if there is\n> > a big chance that those tools will not work.\n> \n> If that's a motivation, wouldn't a check in the gitk and git-gui scripts\n> handle it? That would provide an error at run time to the user. This\n> change is about helping the user who builds their own git and then runs it,\n> so if they built git without wish installed and then ran git-gui, they'd get\n> a clear error that wish is missing and could easily install it. It's not\n> needed for the build, so they wouldn't need to rebuild anything.\n\nI think the message is already OK:\n\n $ ./gitk\n ./gitk: 3: exec: wish: not found\n\nThe question is whether we would want to catch this at build time. And I\nthink Junio's point is that we don't _know_ it's an error at build time.\nWe could be building gitk for use on a system that isn't quite like the\nbuild system, so any \"solution\" here is going to have to make an\nassumption either way.\n\nIt's also not foolproof. You could build when wish is present, and then\nlater uninstall it and receive the same error message.\n\nI also think all of this is largely orthogonal to gettext. It just so\nhappens that if you don't have gettext installed, we'll try to run wish\nas part of the build process, but detecting broken tcl setups was\ndefinitely not part of the intent there.\n\nAnd the failure actually runs the other way, too. If you have neither\ngettext nor tcl, you get this confusing output:\n\n $ make NO_GETTEXT=1\n ...\n MSGFMT po/pt_pt.msg MSGFMT po/hu.msg Makefile:252: recipe for target 'po/pt_pt.msg' failed\n make[1]: *** [po/pt_pt.msg] Error 127\n\n(the problem is not msgfmt, but our tcl substitute which cannot run).\n\nI'm actually tempted to say that we should not be building the tcl parts\nby default. IOW, instead of NO_TCLTK we should have USE_TCLTK. That\nwould also require an adjustment by package builders, but it would\nhopefully be a really obvious one. And once the user has told our\nMakefile that they definitely want to build the tcl parts, we'd\npresumably just trust that the tcl path they give us is sane.\n\nBut it's possible I'm underestimating how many people actually use the\ntcl scripts. Certainly I don't, and git-gui seems fairly primitive to me\nthese days compared to 3rd party tools. But then I don't use any of them\neither. ;)\n\n-Peff\n"},{"id":"332918","messageId":"CAP8UFD3LhR+FRMM--SQVZ_m2-g2w-WkmmFX7aqrp9S5A9vi1ow@mail.gmail.com","threadId":"47212","inReplyTo":"20171117220210.6xqi26mabbyvxc2m@sigill.intra.peff.net","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-11-20T17:25:36Z","receivedAt":"2017-11-20T17:25:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Nov 17, 2017 at 11:02 PM, Jeff King wrote:\n>\n> I'm actually tempted to say that we should not be building the tcl parts\n> by default. IOW, instead of NO_TCLTK we should have USE_TCLTK. That\n> would also require an adjustment by package builders, but it would\n> hopefully be a really obvious one. And once the user has told our\n> Makefile that they definitely want to build the tcl parts, we'd\n> presumably just trust that the tcl path they give us is sane.\n>\n> But it's possible I'm underestimating how many people actually use the\n> tcl scripts. Certainly I don't, and git-gui seems fairly primitive to me\n> these days compared to 3rd party tools. But then I don't use any of them\n> either. ;)\n\nAs I wrote in the version 2 of the patch (yeah I forgot to mark it as\nversion 2) I agree that removing NO_TCLTK while introducing USE_TCLTK\nand not building git-gui and gitk by default is simpler and probably\nthe right direction for the future, but I think it might be too big a\nchange for now or until the next major release (Git 3.0.0).\n\nEspecially it can cause issues to people who were building and\ninstalling gitk and git-gui because they use it. They might not\nrealize that USE_TCLTK has been introduced and still think that they\nare building and installing the newest versions, when in fact they\nwould not and keep using old versions.\n"},{"id":"332923","messageId":"CAP8UFD23ALBQO=kfeKrf4YXFigtC79mLtnBuXtTgjbeqPFE76Q@mail.gmail.com","threadId":"47212","inReplyTo":"20171117174258.GP3693@zaya.teonanacatl.net","subject":"Re: [PATCH] Makefile: check that tcl/tk is installed","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-11-20T18:12:33Z","receivedAt":"2017-11-20T18:12:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Nov 17, 2017 at 6:42 PM, Todd Zullinger wrote:\n> Christian Couder wrote:\n>>\n>> On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano wrote:\n>>>\n>>> I suspect that this change will hurt those who package Git for other\n>>> people.\n>>\n>>\n>> Maybe a little bit, but in my opinion it should not be a big problem for\n>> them to install Tcl/Tk and its dependencies on the build machine.\n>\n> It's not a big burden, but it is a seemingly unnecessary build-time\n> dependency.\n>\n>>> It used to be that, as long as they have msgfmt installed, they only\n>>> needed to _know_ what the path on the users' box to \"wish\" is, and set it to\n>>> TCLTK_PATH, and if they are distro packagers, most likely they already have\n>>> such an automated set-up working. Now with this change, they are forced to\n>>> install tcl/tk on their possibly headless box where tcl/tk is useless, and\n>>> worse yet, an attempt to install it may bring in tons of unwanted stuff\n>>> related to X that is irrelevant on such a headless development environment.\n>>\n>> Yeah, but if they build gitk and git-gui, there is a significant chance\n>> that they build other graphical software too, and that this will require\n>> installing stuff related to X anyway.\n>\n> Most distributions build packages in individual container or chroots, to\n> increase the stability and reproducibility of the builds. So package builds\n> don't run on systems where any deps have already been installed.\n>\n> To be fair, it looks like pulling in tcl/tk would add only around 8MB to the\n> Fedora build root for git. That's not egregious, to be sure. But it really\n> isn't a necessary build-time dependency either. I don't know if there are\n> other distros who would strongly object to pulling in tcl/tk. Some are much\n> more sensitive to build root sizes and unnecessary dependencies.\n\nYeah, I still think that when packaging graphical tools, packagers\nshould be used to managing builds that need a lot of dependencies\n(especially X related dependencies). I used to be a KDE developer in a\nprevious life and the amount of dependencies to build KDE was much\nlarger than what is required for everything in the git repo (git,\ngitk, git-gui, git-svn, etc).\n\n>> In general I think packagers are much more able to deal with those kinds\n>> of problems than most regular developers who want to hack on Git.\n>\n> I agree. Packagers also provide git builds to the vast majority of\n> end-users, so we should make their task easier whenever possible. :)\n\nYeah, but you might have noticed that such checks might be a good\nthing for packagers, as it makes the build fail right away with a\nclear error message. So in the long run, I think this kind of patches\nwill make it easier also for packagers.\n\n>> So asking packagers to either set NO_TCLTK or BYPASS_TCLTK_CHECK or to\n>> install Tcl/Tk would not burden them much, especially compared to what\n>> regular developers have to deal with these days when trying to build Git.\n>\n> Presuming this new BYPASS_TCLTK_CHECK is communicated well and that the\n> failure when not using it is clear, this doesn't seem likely to cause\n> problems.\n\nYeah I agree.\n\n> (I'll leave it to others whether there's a better way to solve\n> the msgfmt fallthrough issue. I didn't even know such a fallthrough existed\n> until yesterday.)\n\nI might also send a similar patch for the msgfmt issue, otherwise it\nmay be a good #leftoverbit for someone starting to hack on Git.\nBut anyway it is a separate issue.\n\n> I think it's important to ensure that automated package builds of a newer\n> git don't simply skip parts of the build which used to work and so packagers\n> reading the failed builds logs can easily see what they need to adjust.\n\nI agree and the patch doesn't skip parts of the build that used to\nwork, and actually it makes logs easier to understand and build\nfailures easier to fix.\n\n> Just dropping the new variable in the Makefile and waiting for package\n> builds to fail or not package gitk & git-gui at the next release would be a\n> bit unkind, I think. Posting this to the git-packagers group[1] which Ævar\n> created would be useful. It /might/ even be worth asking there if any\n> distros have strong opinions on the subject.\n>\n> [1] https://groups.google.com/forum/#!forum/git-packagers and\n> git-packagers@googlegroups.com\n\nI cc'ed this list when I sent version 2 of the patch.\n\n>>> I think \"If I cannot run either wish or msgfmt, then barf and give an\n>>> error message\" might at least be needed. Am I misinterpreting the\n>>> motivation of the patch?\n>>\n>> I'd rather add a separate check for msgfmt than mixing the 2 issues,\n>> because I think that unless it has been explicitly told to do so, Git should\n>> not try to build git-gui and gitk in the first place if there is a big\n>> chance that those tools will not work.\n>\n> If that's a motivation, wouldn't a check in the gitk and git-gui scripts\n> handle it? That would provide an error at run time to the user. This\n> change is about helping the user who builds their own git and then runs it,\n> so if they built git without wish installed and then ran git-gui, they'd get\n> a clear error that wish is missing and could easily install it. It's not\n> needed for the build, so they wouldn't need to rebuild anything.\n\nThis change is not just about people who want to build and run those\ntools, but it already helps them by improving build error messages a\nlot.\n"}]}