On Sun, Nov 26, 2017 at 8:15 PM, Jeff King <peff@xxxxxxxx> 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: >> >> https://public-inbox.org/git/b6b12040-100f-5965-6dfd-344c84dddf96@xxxxxxxx/ >> >> and here are discussions related to version 1 of this patch: >> >> https://public-inbox.org/git/20171115125200.17006-1-chriscool@xxxxxxxxxxxxx/ >> >> As Peff mentions in the original discussion, at the Bloomberg Git >> sprint, we saw someone struggling to compile Git, because of these >> msgfmt and Tcl/Tk issues. > > Actually, I think we had the _opposite_ problem there. > > The main problem your patch fixes is that we may silently build a > version of gitk/git-gui that do not work. The "make" process completes, > but they refer to a non-existent "wish" tool, and running them will > fail. > > That's potentially annoying if you wanted those tools. But if you didn't > care about them in the first place, it's fine. I think it's a bit better to not install the tools if you don't care about them. So overall whether you care or not about them, there is still a bit of improvement. > The opposite problem is when you don't care about those tools, and they > _do_ break the build. And then just to get the rest of Git built, you > have to know about and set NO_TCLTK. > > AFAIK that only happens if you don't have msgfmt installed. Because then > the gitk and git-gui Makefiles try to auto-fallback to implementing > msgfmt in tcl _during the build_, and there a lack of "tclsh" will break > the build. > > I think your patch does say "consider setting NO_TCLTK" in that case, > which is an improvement. Yeah, so my patch actually improve things in all the cases. > 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). Yeah, it might be nicer if it just worked, but as I already answered in another thread, it could break some environments if we just stopped installing gitk and git-gui by default. About improving the way msgfmt is handled, I am not against it. In fact I might even give it a try, but I think it is a separate issue, and I don't want to mix those issues right now. > 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 agree that it doesn't solve all the issues, if you are trying to build git for the first time, but I do think that it makes it easier. If you don't have msgfmt, you get an error that is not so difficult to debug. > I do also wonder if we > want to start putting these kind of run-time checks into the Makefile > itself. That's kind of what autoconf is for. I don't quite agree that autoconf is for that, and there are already some checks in the Makefile. > 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 easier to advice to do as most of us do, and to just add a few checks in the Makefile to make it clear which dependencies should be installed or which knob should be tweaked. > If there are deficiencies in configure.in (and I can well believe that > there are), should we be fixing it there? If most of us don't use autoconf, even if we fix the current deficiencies, there could still be some a few years from now. While if we add checks to the Makefile, there is a good chance that those who change the Makefile will see the existing tests and add more if necessary.
