Christian Couder <christian.couder@xxxxxxxxx> writes:
> To improve the current behavior when Tcl/Tk is not installed,
> let's just check that TCLTK_PATH points to something and error
> out right away if this is not the case.
>
> This should benefit people who actually want to install and use
> git-gui or gitk as they will have to install Tcl/Tk anyway, and
> it is better for them if they are told about installing it soon
> in the build process, rather than if they have to debug it after
> installing.
Not objecting, but thinking aloud if this change makes sense.
If you are building Git for your own use on the same box, which is
presumably the majority of "build failed and I have no clue how to
fix" case that needs help, if you want gui tools, you need to have
tcl/tk installed anyway, whether you have msgfmt installed. This
seems to be the _only_ class of users this patch wants to cater to.
I wonder if we are hurting people who are not in that category.
- To run gui tools, tcl/tk must be available at runtime, but tcl/tk
is not necessary in the packager's environment to produce a
package of Git that contains working git-gui and gitk that will
be used on an end-user box with tcl/tk installed, as long as the
packager's environment has a working msgfmt.
- To process .po files for the gui tools in the packager's
environment without msgfmt, tcl/tk is required.
I suspect that this change will hurt those who package Git for other
people.
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.
I doubt that this is quite a good trade-off; it feels that this
burdens packagers a bit too much, and we may need a way to override
this new check further. 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?
> diff --git a/Makefile b/Makefile
> index ee9d5eb11e..ada6164e15 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1636,6 +1636,13 @@ ifeq ($(TCLTK_PATH),)
> NO_TCLTK = NoThanks
> endif
>
> +ifndef NO_TCLTK
> + has_tcltk := $(shell type $(TCLTK_PATH) 2>/dev/null)
> + ifndef has_tcltk
> +$(error "Tcl/Tk is not installed ('$(TCLTK_PATH)' not found). Consider setting NO_TCLTK or installing Tcl/Tk")
> + endif
> +endif
> +
> ifeq ($(PERL_PATH),)
> NO_PERL = NoThanks
> endif
