[PATCH] git-gui: Build even if tclsh is not available

[PATCH] git-gui: Build even if tclsh is not available

[Petr Baudis]

As of now, git fails to build with default config if tclsh is not
available, thus requiring manual config tweaking just because of some
optimizations done at build time; this is a needless hassle when building
git on any kind of a server.

This patch makes git-gui's build process to handle this gracefully; unless
TCL_PATH is explicitly set, if tclsh cannot be executed only a warning is
printed and the build goes on. I have tested this only on that server with
no tclsh, but hopefully it shouldn't break the build process with tclsh
available either.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile         |    8 +++++---
 git-gui/Makefile |    2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 07411ff..ed12577 100644
--- a/Makefile
+++ b/Makefile
@@ -178,10 +178,12 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
-TCL_PATH = tclsh
-TCLTK_PATH = wish
 
-export TCL_PATH TCLTK_PATH
+# If TCL_PATH is not defined here, it will default to tclsh later
+# with the exception that if tclsh cannot be executed, the optimization
+# step is skipped.
+# export TCL_PATH = tclsh
+export TCLTK_PATH = wish
 
 # sparse is architecture-neutral, which means that we need to tell it
 # explicitly what architecture to check for. Fix this up for yours..
diff --git a/git-gui/Makefile b/git-gui/Makefile
index e73b645..2b1bc18 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -34,7 +34,7 @@ ifndef V
 	QUIET_INDEX    = @echo '   ' INDEX $(dir $@);
 endif
 
-TCL_PATH   ?= tclsh
+TCL_PATH   ?= tclsh || echo "Warning: Cannot execute tclsh, not optimizing git-gui" >&2
 TCLTK_PATH ?= wish
 
 ifeq ($(findstring $(MAKEFLAGS),s),s)

[PATCH] git-gui: Build even if tclsh is not available

[Petr Baudis]

As of now, git fails to build with default config if tclsh is not
available, thus requiring manual config tweaking just because of some
optimizations done at build time; this is a needless hassle when building
git on any kind of a server.

This patch makes git-gui's build process to handle this gracefully; unless
TCL_PATH is explicitly set, if tclsh cannot be executed only a warning is
printed and the build goes on. I have tested this only on that server with
no tclsh, but hopefully it shouldn't break the build process with tclsh
available either.

Version 2, sorry - the previous version of the patch had the install hunk
missing. And apologies for the duplicate submission... :-)

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile         |    8 +++++---
 git-gui/Makefile |    4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 07411ff..ed12577 100644
--- a/Makefile
+++ b/Makefile
@@ -178,10 +178,12 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
-TCL_PATH = tclsh
-TCLTK_PATH = wish
 
-export TCL_PATH TCLTK_PATH
+# If TCL_PATH is not defined here, it will default to tclsh later
+# with the exception that if tclsh cannot be executed, the optimization
+# step is skipped.
+# export TCL_PATH = tclsh
+export TCLTK_PATH = wish
 
 # sparse is architecture-neutral, which means that we need to tell it
 # explicitly what architecture to check for. Fix this up for yours..
diff --git a/git-gui/Makefile b/git-gui/Makefile
index e73b645..be3cfbb 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -34,7 +34,7 @@ ifndef V
 	QUIET_INDEX    = @echo '   ' INDEX $(dir $@);
 endif
 
-TCL_PATH   ?= tclsh
+TCL_PATH   ?= tclsh || echo "Warning: Cannot execute tclsh, not optimizing git-gui" >&2
 TCLTK_PATH ?= wish
 
 ifeq ($(findstring $(MAKEFLAGS),s),s)
@@ -92,7 +92,7 @@ install: all
 	$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'
-	$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
+	[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
 	$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)
 
 dist-version:

Re: [PATCH] git-gui: Build even if tclsh is not available

[Shawn O. Pearce]

Petr Baudis <pasky@suse.cz> wrote:
> As of now, git fails to build with default config if tclsh is not
> available, thus requiring manual config tweaking just because of some
> optimizations done at build time; this is a needless hassle when building
> git on any kind of a server.
> 
> This patch makes git-gui's build process to handle this gracefully; unless
> TCL_PATH is explicitly set, if tclsh cannot be executed only a warning is
> printed and the build goes on. I have tested this only on that server with
> no tclsh, but hopefully it shouldn't break the build process with tclsh
> available either.
> 
> Version 2, sorry - the previous version of the patch had the install hunk
> missing. And apologies for the duplicate submission... :-)
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
> 
>  Makefile         |    8 +++++---
>  git-gui/Makefile |    4 ++--
>  2 files changed, 7 insertions(+), 5 deletions(-)

I have a couple of problems with the patch as-is.  The first is
of course that the patch needs to be split into two; one patch for
the git-gui subdirectory itself and one for git.git.

My other problem is 

>  ifeq ($(findstring $(MAKEFLAGS),s),s)
> @@ -92,7 +92,7 @@ install: all
>  	$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'
>  	$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
>  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'
> -	$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
> +	[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
>  	$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)

git-gui won't work if lib/tclIndex is missing or invalid.  So not
installing it means we should just disable git-gui entirely.

-- 
Shawn.

Re: [PATCH] git-gui: Build even if tclsh is not available

[Petr Baudis]

On Thu, May 17, 2007 at 04:18:58AM CEST, Shawn O. Pearce wrote:
> I have a couple of problems with the patch as-is.  The first is
> of course that the patch needs to be split into two; one patch for
> the git-gui subdirectory itself and one for git.git.

Hmm, why? It's an atomic change, one part doesn't make sense without the
other.

> My other problem is 
> 
> >  ifeq ($(findstring $(MAKEFLAGS),s),s)
> > @@ -92,7 +92,7 @@ install: all
> >  	$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'
> >  	$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
> >  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'
> > -	$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
> > +	[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'
> >  	$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)
> 
> git-gui won't work if lib/tclIndex is missing or invalid.  So not
> installing it means we should just disable git-gui entirely.

Aha, ouch - I understood that it is only an optimization. :-(

So AIUI, there are several possibilities:

  (i) Makefile will autodecide on whether git-gui will be
built+installed or not

  (ii) ./configure will, people not using configure and building on
servers will be left to tweak config manually

  (iii) ./configure will, git-gui will default to not to be built and
people not using configure and wanting git-gui will be left to tweak
config manually

I suspect that (ii) will be chosen, and even though I don't like it
*personally* I guess it's the most reasonable approach for the general
public. I didn't know that tclIndex is vital for git-gui when I
submitted the patch, the /Makefile comment suggests otherwise.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH] git-gui: Build even if tclsh is not available

[Shawn O. Pearce]

Petr Baudis <pasky@suse.cz> wrote:
> On Thu, May 17, 2007 at 04:18:58AM CEST, Shawn O. Pearce wrote:
> > I have a couple of problems with the patch as-is.  The first is
> > of course that the patch needs to be split into two; one patch for
> > the git-gui subdirectory itself and one for git.git.
> 
> Hmm, why? It's an atomic change, one part doesn't make sense without the
> other.

Because git-gui is actually a project maintained external from
git.git.  It just happens that Junio pulls various versions of
it into git.git to distribute it along with git.git releases.

I deal with these sorts of "atomic changes" by making an evil merge
in git.git and asking Junio to pull the evil merge.  But we've only
had one such case thus far.
 
> > git-gui won't work if lib/tclIndex is missing or invalid.  So not
> > installing it means we should just disable git-gui entirely.
> 
>   (i) Makefile will autodecide on whether git-gui will be
> built+installed or not
> 
>   (ii) ./configure will, people not using configure and building on
> servers will be left to tweak config manually
> 
>   (iii) ./configure will, git-gui will default to not to be built and
> people not using configure and wanting git-gui will be left to tweak
> config manually

(iv) if tclsh is not available then create a simpler lib/tclIndex that
loads all of the lib directory, even if it isn't needed.  That makes
tclsh being available strictly an optimization, and yet git-gui still
is installable.

-- 
Shawn.

Re: [PATCH] git-gui: Build even if tclsh is not available

[Shawn O. Pearce]

Petr Baudis <pasky@suse.cz> wrote:
>   (i) Makefile will autodecide on whether git-gui will be
> built+installed or not
> 
>   (ii) ./configure will, people not using configure and building on
> servers will be left to tweak config manually
> 
>   (iii) ./configure will, git-gui will default to not to be built and
> people not using configure and wanting git-gui will be left to tweak
> config manually
> 
> I suspect that (ii) will be chosen, and even though I don't like it
> *personally* I guess it's the most reasonable approach for the general
> public. I didn't know that tclIndex is vital for git-gui when I
> submitted the patch, the /Makefile comment suggests otherwise.

(iv) git-gui 0.7.1, which is now available from repo.or.cz, and
I think you know where that is ;-), makes TCL_PATH strictly an
optimization at compile time.

If TCL_PATH is present and creates a valid tclIndex file then we
use the Tcl auto_load "optimization" to only load the Tcl code we
actually need when we run.  But if it does fail for any reason we
hide the error (can be unhidden with make V=1) and we generate
a listing of the files instead.  In this latter case we load
*everything* on git-gui startup.

This way git-gui works for the user either way, just like before,
but its startup may be slightly slower if the user didn't give us
a good TCL_PATH at build time.

Junio, would you consider merging 0.7.1 soon?  No changes are
needed to git.git's own Makefile, just a subtree pull.

-- 
Shawn.

[PATCH] git-gui: Build even if tclsh is not available

[Petr Baudis]

As of now, git fails to build with default config if tclsh is not
available, thus requiring manual config tweaking just because of some
optimizations done at build time; this is a needless hassle when building
git on any kind of a server.

This patch makes git-gui's build process to handle this gracefully; unless
TCL_PATH is explicitly set, if tclsh cannot be executed only a warning is
printed and the build goes on. I have tested this only on that server with
no tclsh, but hopefully it shouldn't break the build process with tclsh
available either.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Makefile         |    8 +++++---
 git-gui/Makefile |    2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 07411ff..ed12577 100644
--- a/Makefile
+++ b/Makefile
@@ -178,10 +178,12 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
-TCL_PATH = tclsh
-TCLTK_PATH = wish
 
-export TCL_PATH TCLTK_PATH
+# If TCL_PATH is not defined here, it will default to tclsh later
+# with the exception that if tclsh cannot be executed, the optimization
+# step is skipped.
+# export TCL_PATH = tclsh
+export TCLTK_PATH = wish
 
 # sparse is architecture-neutral, which means that we need to tell it
 # explicitly what architecture to check for. Fix this up for yours..
diff --git a/git-gui/Makefile b/git-gui/Makefile
index e73b645..2b1bc18 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -34,7 +34,7 @@ ifndef V
 	QUIET_INDEX    = @echo '   ' INDEX $(dir $@);
 endif
 
-TCL_PATH   ?= tclsh
+TCL_PATH   ?= tclsh || echo "Warning: Cannot execute tclsh, not optimizing git-gui" >&2
 TCLTK_PATH ?= wish
 
 ifeq ($(findstring $(MAKEFLAGS),s),s)