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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Couder wrote:
On Thu, Nov 16, 2017 at 2:35 AM, Junio C Hamano <gitster@xxxxxxxxx> 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@xxxxxxxxxxxxxxxx

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux