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

[Todd Zullinger <tmz@pobox.com>]

Christian Couder wrote:
> On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano <gitster@pobox.com> 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 on the build machine.

It's not a big burden, but it is a seemingly unnecessary build-time 
dependency.

>> It used to be that, as long as they have msgfmt installed, they only 
>> needed to _know_ what the path on the users' box to "wish" is, and 
>> set it to TCLTK_PATH, and if they are distro packagers, most likely 
>> they already have such an automated set-up working.  Now with this 
>> change, they are forced to install tcl/tk on their possibly headless 
>> box where tcl/tk is useless, and worse yet, an attempt to install it 
>> may bring in tons of unwanted stuff related to X that is irrelevant 
>> on such a headless development environment.
>
> Yeah, but if they build gitk and git-gui, there is a significant 
> chance that they build other graphical software too, and that this 
> will require installing stuff related to X anyway.

Most distributions build packages in individual container or chroots, 
to increase the stability and reproducibility of the builds.  So 
package builds don't run on systems where any deps have already been 
installed.

To be fair, it looks like pulling in tcl/tk would add only around 8MB 
to the Fedora build root for git.  That's not egregious, to be sure.  
But it really isn't a necessary build-time dependency either.  I don't 
know if there are other distros who would strongly object to pulling 
in tcl/tk.  Some are much more sensitive to build root sizes and 
unnecessary dependencies.

> In general I think packagers are much more able to deal with those 
> kinds of problems than most regular developers who want to hack on 
> Git.

I agree.  Packagers also provide git builds to the vast majority of 
end-users, so we should make their task easier whenever possible. :)

> So asking packagers to either set NO_TCLTK or BYPASS_TCLTK_CHECK or to 
> install Tcl/Tk would not burden them much, especially compared to what 
> regular developers have to deal with these days when trying to build 
> Git.

Presuming this new BYPASS_TCLTK_CHECK is communicated well and that 
the failure when not using it is clear, this doesn't seem likely to 
cause problems.  (I'll leave it to others whether there's a better way 
to solve the msgfmt fallthrough issue.  I didn't even know such a 
fallthrough existed until yesterday.)

I think it's important to ensure that automated package builds of a 
newer git don't simply skip parts of the build which used to work and 
so packagers reading the failed builds logs can easily see what they 
need to adjust.

Just dropping the new variable in the Makefile and waiting for package 
builds to fail or not package gitk & git-gui at the next release would 
be a bit unkind, I think.  Posting this to the git-packagers group[1] 
which Ævar created would be useful.  It /might/ even be worth asking 
there if any distros have strong opinions on the subject.

[1] https://groups.google.com/forum/#!forum/git-packagers and
    git-packagers@googlegroups.com

>> I think "If I cannot run either wish or msgfmt, then barf and give 
>> an error message" might at least be needed.  Am I misinterpreting 
>> the motivation of the patch?
>
> 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 chance that those tools will not work.

If that's a motivation, wouldn't a check in the gitk and git-gui 
scripts handle it?  That would provide an error at run time to the 
user.  This change is about helping the user who builds their own git 
and then runs it, so if they built git without wish installed and then 
ran git-gui, they'd get a clear error that wish is missing and could 
easily install it.  It's not needed for the build, so they wouldn't 
need to rebuild anything.

Something like this:

diff --git i/gitk-git/gitk w/gitk-git/gitk
index a14d7a16b2..f9f28a164a 100755
--- i/gitk-git/gitk
+++ w/gitk-git/gitk
@@ -1,5 +1,6 @@
 #!/bin/sh
-# Tcl ignores the next line -*- tcl -*- \
+# Tcl ignores the next 2 lines -*- tcl -*- \
+type wish >/dev/null 2>&1 || { echo "error: wish not found" >&2; exit 1; }; \
 exec wish "$0" -- "$@"
 
 # Copyright © 2005-2016 Paul Mackerras.  All rights reserved.

maybe?  (The error message is certainly open for improvement.)

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A fool's brain digests philosophy into folly, science into
superstition, and art into pedantry.  Hence University education.
    -- George Bernard Shaw