Christian Couder [off-list ref] writes:
On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
[off-list ref] wrote:
quoted
By default running `make install` in the root directory of the
project will set TCLTK_PATH to `wish` and then go into the "git-gui"
and "gitk-git" sub-directories to build and install these 2
sub-projects.
Has this patch fallen through the cracks or is there an unresolved issue?
I had an impression that the conclusion was that the existing error
message at runtime already does an adequate job and there is no
issue to be addressed by this patch. Am I mistaken?
On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano [off-list ref] wrote:
Christian Couder [off-list ref] writes:
quoted
On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
[off-list ref] wrote:
quoted
By default running `make install` in the root directory of the
project will set TCLTK_PATH to `wish` and then go into the "git-gui"
and "gitk-git" sub-directories to build and install these 2
sub-projects.
Has this patch fallen through the cracks or is there an unresolved issue?
I had an impression that the conclusion was that the existing error
message at runtime already does an adequate job and there is no
issue to be addressed by this patch. Am I mistaken?
This patch is mostly about what happens at the build step. Its goal is
not much to improve what happens at runtime, though that is improved a
bit too. If the build step was good enough, then I would agree that
what happens at run time is adequate.
Let's consider only people installing git using "make install" to use
it on their machine, as I think I already discussed the case of
packagers and added the BYPASS_TCLTK_CHECK variable for them.
When those users run "make install", let's suppose they don't have
Tcl/Tk installed. (If it is already installed my patch changes
nothing.)
Let's also suppose that NO_TCLTK is not set. (If it is set my patch
changes nothing.)
Then there are 2 cases:
- msgfmt is already installed
Without my patch, the "make install" step works and installs git,
git-gui and gitk. But the user might not want to have git-gui and gitk
installed in the first place. As Jeff says there are a lot of other
graphical tools, that are more advanced these days, so there is a big
chance that they just forgot to set NO_TCLTK or just didn't know about
this variable. Now if they actually want to use git-gui and gitk, they
will get an error at run time (which is adequate) and they will have
to install Tck/Tk then before they can git-gui and gitk.
With my patch, the "make install" step fails right away telling them
to either set NO_TCLTK or install Tcl/Tk. If they don't want to use
git-gui and gitk, they will set NO_TCLTK and then run "make install"
again and things will be exactly as they want. If they do want to use
git-gui and gitk, they will install Tcl/Tk and then run "make install"
again and then things will be exactly as they want and there will be
no error at runtime.
So my opinion is that whether they actually want to use git-gui and
gitk or not, forcing them to decide is probably a good thing because
it ensures that when "make install" succeeds things are exactly how
they want them to be. The only case when it might not be a good thing
is if they don't know yet if they will actually want to use gitk and
git-gui. But even in this case, which is not the most common, they are
not much worse.
- msgfmt is not installed
Without my patch, the "make install" step produces an error message
while building git-gui. Debugging and fixing the error is quite
difficult especially for new comers. They will have to either set
NO_GETTEXT or install gettext, and to either set NO_TCLTK or to
install Tcl/Tk before the build can fully succeed.
With my patch, the "make install" step fails right away telling them
to either set NO_TCLTK or install Tcl/Tk. Then the build will fail
again and they will have to either set NO_GETTEXT or install gettext,
but this error is easier to understand and to fix than without my
patch.
On 26/11/17 14:00, Christian Couder wrote:
On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano [off-list ref] wrote:
quoted
Christian Couder [off-list ref] writes:
quoted
On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
[off-list ref] wrote:
quoted
By default running `make install` in the root directory of the
project will set TCLTK_PATH to `wish` and then go into the "git-gui"
and "gitk-git" sub-directories to build and install these 2
sub-projects.
Has this patch fallen through the cracks or is there an unresolved issue?
I had an impression that the conclusion was that the existing error
message at runtime already does an adequate job and there is no
issue to be addressed by this patch. Am I mistaken?
This patch is mostly about what happens at the build step. Its goal is
not much to improve what happens at runtime, though that is improved a
bit too. If the build step was good enough, then I would agree that
what happens at run time is adequate.
Let's consider only people installing git using "make install" to use
it on their machine, as I think I already discussed the case of
packagers and added the BYPASS_TCLTK_CHECK variable for them.
I haven't been following this thread too closely, but I have the
feeling that the best course of action is to simply not fall back
to using a tcl version of msgfmt in the first place!. ;-)
If a given platform does not have gettext/msgfmt, then you just
don't get an i18n-ed version of git. (no need for BYPASS_ ...).
Am I missing something?
ATB,
Ramsay Jones
On Sun, Nov 26, 2017 at 6:43 PM, Ramsay Jones
[off-list ref] wrote:
On 26/11/17 14:00, Christian Couder wrote:
quoted
On Sun, Nov 26, 2017 at 4:53 AM, Junio C Hamano [off-list ref] wrote:
quoted
Christian Couder [off-list ref] writes:
quoted
On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
[off-list ref] wrote:
quoted
By default running `make install` in the root directory of the
project will set TCLTK_PATH to `wish` and then go into the "git-gui"
and "gitk-git" sub-directories to build and install these 2
sub-projects.
Has this patch fallen through the cracks or is there an unresolved issue?
I had an impression that the conclusion was that the existing error
message at runtime already does an adequate job and there is no
issue to be addressed by this patch. Am I mistaken?
This patch is mostly about what happens at the build step. Its goal is
not much to improve what happens at runtime, though that is improved a
bit too. If the build step was good enough, then I would agree that
what happens at run time is adequate.
Let's consider only people installing git using "make install" to use
it on their machine, as I think I already discussed the case of
packagers and added the BYPASS_TCLTK_CHECK variable for them.
I haven't been following this thread too closely, but I have the
feeling that the best course of action is to simply not fall back
to using a tcl version of msgfmt in the first place!. ;-)
Well, another possibility would be to try to use the tcl version of
msgfmt in the build of git itself if msgfmt is not available.
This way the build behavior of git and git-gui could be similar.
If a given platform does not have gettext/msgfmt, then you just
don't get an i18n-ed version of git. (no need for BYPASS_ ...).
Right now without gettext/msgfmt you get an error unless you set
NO_GETTEXT. If we try to use the tcl version of msgfmt in the build of
git itself, then you could still get an i18n-ed version of git if you
have Tcl/Tk.
But anyway even if this is related, I think it is a different issue.
Am I missing something?
I still think it is not the best outcome to just install git-gui and
gitk by default when Tcl/Tk is not installed. In general it is best to
fix potential errors at build time rather than at run time (even if
the run time error is adequate).