{"thread":{"id":"8183","subject":"[PATCH] git-gui: Build even if tclsh is not available","startedAt":"2007-05-17T02:06:16Z","lastAt":"2007-05-17T22:16:15Z","messageCount":6,"participants":["Petr Baudis","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42355","messageId":"20070517020616.4722.33946.stgit@rover","threadId":"8183","inReplyTo":null,"subject":"[PATCH] git-gui: Build even if tclsh is not available","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-05-17T02:06:16Z","receivedAt":"2007-05-17T02:06:16Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"As of now, git fails to build with default config if tclsh is not\navailable, thus requiring manual config tweaking just because of some\noptimizations done at build time; this is a needless hassle when building\ngit on any kind of a server.\n\nThis patch makes git-gui's build process to handle this gracefully; unless\nTCL_PATH is explicitly set, if tclsh cannot be executed only a warning is\nprinted and the build goes on. I have tested this only on that server with\nno tclsh, but hopefully it shouldn't break the build process with tclsh\navailable either.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n Makefile         |    8 +++++---\n git-gui/Makefile |    2 +-\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 07411ff..ed12577 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -178,10 +178,12 @@ AR = ar\n TAR = tar\n INSTALL = install\n RPMBUILD = rpmbuild\n-TCL_PATH = tclsh\n-TCLTK_PATH = wish\n \n-export TCL_PATH TCLTK_PATH\n+# If TCL_PATH is not defined here, it will default to tclsh later\n+# with the exception that if tclsh cannot be executed, the optimization\n+# step is skipped.\n+# export TCL_PATH = tclsh\n+export TCLTK_PATH = wish\n \n # sparse is architecture-neutral, which means that we need to tell it\n # explicitly what architecture to check for. Fix this up for yours..\ndiff --git a/git-gui/Makefile b/git-gui/Makefile\nindex e73b645..2b1bc18 100644\n--- a/git-gui/Makefile\n+++ b/git-gui/Makefile\n@@ -34,7 +34,7 @@ ifndef V\n \tQUIET_INDEX    = @echo '   ' INDEX $(dir $@);\n endif\n \n-TCL_PATH   ?= tclsh\n+TCL_PATH   ?= tclsh || echo \"Warning: Cannot execute tclsh, not optimizing git-gui\" >&2\n TCLTK_PATH ?= wish\n \n ifeq ($(findstring $(MAKEFLAGS),s),s)\n"},{"id":"42357","messageId":"20070517021448.24022.8282.stgit@rover","threadId":"8183","inReplyTo":"20070517020616.4722.33946.stgit@rover","subject":"[PATCH] git-gui: Build even if tclsh is not available","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-05-17T02:14:48Z","receivedAt":"2007-05-17T02:14:48Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"As of now, git fails to build with default config if tclsh is not\navailable, thus requiring manual config tweaking just because of some\noptimizations done at build time; this is a needless hassle when building\ngit on any kind of a server.\n\nThis patch makes git-gui's build process to handle this gracefully; unless\nTCL_PATH is explicitly set, if tclsh cannot be executed only a warning is\nprinted and the build goes on. I have tested this only on that server with\nno tclsh, but hopefully it shouldn't break the build process with tclsh\navailable either.\n\nVersion 2, sorry - the previous version of the patch had the install hunk\nmissing. And apologies for the duplicate submission... :-)\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n Makefile         |    8 +++++---\n git-gui/Makefile |    4 ++--\n 2 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 07411ff..ed12577 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -178,10 +178,12 @@ AR = ar\n TAR = tar\n INSTALL = install\n RPMBUILD = rpmbuild\n-TCL_PATH = tclsh\n-TCLTK_PATH = wish\n \n-export TCL_PATH TCLTK_PATH\n+# If TCL_PATH is not defined here, it will default to tclsh later\n+# with the exception that if tclsh cannot be executed, the optimization\n+# step is skipped.\n+# export TCL_PATH = tclsh\n+export TCLTK_PATH = wish\n \n # sparse is architecture-neutral, which means that we need to tell it\n # explicitly what architecture to check for. Fix this up for yours..\ndiff --git a/git-gui/Makefile b/git-gui/Makefile\nindex e73b645..be3cfbb 100644\n--- a/git-gui/Makefile\n+++ b/git-gui/Makefile\n@@ -34,7 +34,7 @@ ifndef V\n \tQUIET_INDEX    = @echo '   ' INDEX $(dir $@);\n endif\n \n-TCL_PATH   ?= tclsh\n+TCL_PATH   ?= tclsh || echo \"Warning: Cannot execute tclsh, not optimizing git-gui\" >&2\n TCLTK_PATH ?= wish\n \n ifeq ($(findstring $(MAKEFLAGS),s),s)\n@@ -92,7 +92,7 @@ install: all\n \t$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'\n \t$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)\n \t$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'\n-\t$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n+\t[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n \t$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)\n \n dist-version:\n"},{"id":"42359","messageId":"20070517021858.GY3141@spearce.org","threadId":"8183","inReplyTo":"20070517021448.24022.8282.stgit@rover","subject":"Re: [PATCH] git-gui: Build even if tclsh is not available","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-17T02:18:58Z","receivedAt":"2007-05-17T02:18:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Petr Baudis <pasky@suse.cz> wrote:\n> As of now, git fails to build with default config if tclsh is not\n> available, thus requiring manual config tweaking just because of some\n> optimizations done at build time; this is a needless hassle when building\n> git on any kind of a server.\n> \n> This patch makes git-gui's build process to handle this gracefully; unless\n> TCL_PATH is explicitly set, if tclsh cannot be executed only a warning is\n> printed and the build goes on. I have tested this only on that server with\n> no tclsh, but hopefully it shouldn't break the build process with tclsh\n> available either.\n> \n> Version 2, sorry - the previous version of the patch had the install hunk\n> missing. And apologies for the duplicate submission... :-)\n> \n> Signed-off-by: Petr Baudis <pasky@suse.cz>\n> ---\n> \n>  Makefile         |    8 +++++---\n>  git-gui/Makefile |    4 ++--\n>  2 files changed, 7 insertions(+), 5 deletions(-)\n\nI have a couple of problems with the patch as-is.  The first is\nof course that the patch needs to be split into two; one patch for\nthe git-gui subdirectory itself and one for git.git.\n\nMy other problem is \n\n>  ifeq ($(findstring $(MAKEFLAGS),s),s)\n> @@ -92,7 +92,7 @@ install: all\n>  \t$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'\n>  \t$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)\n>  \t$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'\n> -\t$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n> +\t[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n>  \t$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)\n\ngit-gui won't work if lib/tclIndex is missing or invalid.  So not\ninstalling it means we should just disable git-gui entirely.\n\n-- \nShawn.\n"},{"id":"42364","messageId":"20070517023614.GL4489@pasky.or.cz","threadId":"8183","inReplyTo":"20070517021858.GY3141@spearce.org","subject":"Re: [PATCH] git-gui: Build even if tclsh is not available","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-05-17T02:36:14Z","receivedAt":"2007-05-17T02:36:14Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Thu, May 17, 2007 at 04:18:58AM CEST, Shawn O. Pearce wrote:\n> I have a couple of problems with the patch as-is.  The first is\n> of course that the patch needs to be split into two; one patch for\n> the git-gui subdirectory itself and one for git.git.\n\nHmm, why? It's an atomic change, one part doesn't make sense without the\nother.\n\n> My other problem is \n> \n> >  ifeq ($(findstring $(MAKEFLAGS),s),s)\n> > @@ -92,7 +92,7 @@ install: all\n> >  \t$(INSTALL) git-gui '$(DESTDIR_SQ)$(gitexecdir_SQ)'\n> >  \t$(foreach p,$(GITGUI_BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)\n> >  \t$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(libdir_SQ)'\n> > -\t$(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n> > +\t[ ! -e lib/tclIndex ] || $(INSTALL) -m644 lib/tclIndex '$(DESTDIR_SQ)$(libdir_SQ)'\n> >  \t$(foreach p,$(ALL_LIBFILES), $(INSTALL) -m644 $p '$(DESTDIR_SQ)$(libdir_SQ)' ;)\n> \n> git-gui won't work if lib/tclIndex is missing or invalid.  So not\n> installing it means we should just disable git-gui entirely.\n\nAha, ouch - I understood that it is only an optimization. :-(\n\nSo AIUI, there are several possibilities:\n\n  (i) Makefile will autodecide on whether git-gui will be\nbuilt+installed or not\n\n  (ii) ./configure will, people not using configure and building on\nservers will be left to tweak config manually\n\n  (iii) ./configure will, git-gui will default to not to be built and\npeople not using configure and wanting git-gui will be left to tweak\nconfig manually\n\nI suspect that (ii) will be chosen, and even though I don't like it\n*personally* I guess it's the most reasonable approach for the general\npublic. I didn't know that tclIndex is vital for git-gui when I\nsubmitted the patch, the /Makefile comment suggests otherwise.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nEver try. Ever fail. No matter. // Try again. Fail again. Fail better.\n\t\t-- Samuel Beckett\n"},{"id":"42366","messageId":"20070517024911.GZ3141@spearce.org","threadId":"8183","inReplyTo":"20070517023614.GL4489@pasky.or.cz","subject":"Re: [PATCH] git-gui: Build even if tclsh is not available","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-17T02:49:11Z","receivedAt":"2007-05-17T02:49:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Petr Baudis <pasky@suse.cz> wrote:\n> On Thu, May 17, 2007 at 04:18:58AM CEST, Shawn O. Pearce wrote:\n> > I have a couple of problems with the patch as-is.  The first is\n> > of course that the patch needs to be split into two; one patch for\n> > the git-gui subdirectory itself and one for git.git.\n> \n> Hmm, why? It's an atomic change, one part doesn't make sense without the\n> other.\n\nBecause git-gui is actually a project maintained external from\ngit.git.  It just happens that Junio pulls various versions of\nit into git.git to distribute it along with git.git releases.\n\nI deal with these sorts of \"atomic changes\" by making an evil merge\nin git.git and asking Junio to pull the evil merge.  But we've only\nhad one such case thus far.\n \n> > git-gui won't work if lib/tclIndex is missing or invalid.  So not\n> > installing it means we should just disable git-gui entirely.\n> \n>   (i) Makefile will autodecide on whether git-gui will be\n> built+installed or not\n> \n>   (ii) ./configure will, people not using configure and building on\n> servers will be left to tweak config manually\n> \n>   (iii) ./configure will, git-gui will default to not to be built and\n> people not using configure and wanting git-gui will be left to tweak\n> config manually\n\n(iv) if tclsh is not available then create a simpler lib/tclIndex that\nloads all of the lib directory, even if it isn't needed.  That makes\ntclsh being available strictly an optimization, and yet git-gui still\nis installable.\n\n-- \nShawn.\n"},{"id":"42436","messageId":"20070517221615.GA3141@spearce.org","threadId":"8183","inReplyTo":"20070517023614.GL4489@pasky.or.cz","subject":"Re: [PATCH] git-gui: Build even if tclsh is not available","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-17T22:16:15Z","receivedAt":"2007-05-17T22:16:15Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Petr Baudis <pasky@suse.cz> wrote:\n>   (i) Makefile will autodecide on whether git-gui will be\n> built+installed or not\n> \n>   (ii) ./configure will, people not using configure and building on\n> servers will be left to tweak config manually\n> \n>   (iii) ./configure will, git-gui will default to not to be built and\n> people not using configure and wanting git-gui will be left to tweak\n> config manually\n> \n> I suspect that (ii) will be chosen, and even though I don't like it\n> *personally* I guess it's the most reasonable approach for the general\n> public. I didn't know that tclIndex is vital for git-gui when I\n> submitted the patch, the /Makefile comment suggests otherwise.\n\n(iv) git-gui 0.7.1, which is now available from repo.or.cz, and\nI think you know where that is ;-), makes TCL_PATH strictly an\noptimization at compile time.\n\nIf TCL_PATH is present and creates a valid tclIndex file then we\nuse the Tcl auto_load \"optimization\" to only load the Tcl code we\nactually need when we run.  But if it does fail for any reason we\nhide the error (can be unhidden with make V=1) and we generate\na listing of the files instead.  In this latter case we load\n*everything* on git-gui startup.\n\nThis way git-gui works for the user either way, just like before,\nbut its startup may be slightly slower if the user didn't give us\na good TCL_PATH at build time.\n\nJunio, would you consider merging 0.7.1 soon?  No changes are\nneeded to git.git's own Makefile, just a subtree pull.\n\n-- \nShawn.\n"}]}