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
