[PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

NO_GUI disables the building and installation of the git GUI part.
WITH_P4IMPORT enables the installation of the Perforce import script.

This patch was originally developed for the FreeBSD port of git,
but I think that it will not harm to integrate this patch into the
development tree.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 51c1fed..8b142f0 100644
--- a/Makefile
+++ b/Makefile
@@ -110,6 +110,10 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define NO_GUI if you do not want Tcl/Tk GUI.
+#
+# Define WITH_P4IMPORT to build and install Python git-p4import script.
+#
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -196,9 +200,20 @@ SCRIPT_PERL = \
 	git-svnimport.perl git-cvsexportcommit.perl \
 	git-send-email.perl git-svn.perl
 
+SCRIPT_PYTHON = \
+	git-p4import.py
+
+ifdef WITH_P4IMPORT
+SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
+	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
+	  git-status git-instaweb
+else
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	  git-status git-instaweb
+endif
+
 
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS = \
@@ -241,6 +256,9 @@ endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl
 endif
+ifndef PYTHON_PATH
+	PYTHON_PATH = /usr/local/bin/python
+endif
 
 export PERL_PATH
 
@@ -646,6 +664,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
@@ -667,7 +686,9 @@ ifneq (,$X)
 endif
 
 all::
+ifndef NO_GUI
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
+endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
@@ -699,6 +720,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
+$(patsubst %.py,%,$(SCRIPT_PYTHON)) : % : %.py
+	rm -f $@ $@+
+	sed -e '1s|#!.*/python|#!$(PYTHON_PATH_SQ)|' \
+	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    $@.py >$@+
+	chmod +x $@+
+	mv $@+ $@
+
 perl/perl.mak: GIT-CFLAGS
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
 
@@ -892,10 +922,16 @@ install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+ifndef NO_GUI
 	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+else
+	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' install
+ifndef NO_GUI
 	$(MAKE) -C git-gui install
+endif
 	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
@@ -929,11 +965,17 @@ dist: git.spec git-archive
 	@mkdir -p $(GIT_TARNAME)
 	@cp git.spec $(GIT_TARNAME)
 	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
+ifndef NO_GUI
 	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
 	$(TAR) rf $(GIT_TARNAME).tar \
 		$(GIT_TARNAME)/git.spec \
 		$(GIT_TARNAME)/version \
 		$(GIT_TARNAME)/git-gui/version
+else
+	$(TAR) rf $(GIT_TARNAME).tar \
+		$(GIT_TARNAME)/git.spec \
+		$(GIT_TARNAME)/version
+endif
 	@rm -rf $(GIT_TARNAME)
 	gzip -f -9 $(GIT_TARNAME).tar
 
@@ -974,7 +1016,9 @@ clean:
 	rm -f gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
 	$(MAKE) -C perl clean
+ifndef NO_GUI
 	$(MAKE) -C git-gui clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS
-- 
1.5.0.3-dirty
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Jakub Narebski]

Eygene Ryabinkin wrote:

> NO_GUI disables the building and installation of the git GUI part.

By the way, it would be nice for ./configure script (generated from
configure.ac) to detect if Tcl/Tk is available and disable
git-gui and gitk installation if it is not found.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Jakub, good day.

Wed, Mar 21, 2007 at 01:35:22AM +0100, Jakub Narebski wrote:
> Eygene Ryabinkin wrote:
> 
> > NO_GUI disables the building and installation of the git GUI part.
> 
> By the way, it would be nice for ./configure script (generated from
> configure.ac) to detect if Tcl/Tk is available and disable
> git-gui and gitk installation if it is not found.

Will try to implement. But still, NO_GUI should ban the GUI tools
from being built and installed, because user can have the Tcl/Tk
available, but have no intention to use the git GUI. Am I right?
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Johannes Schindelin]

Hi,

On Wed, 21 Mar 2007, Eygene Ryabinkin wrote:

> Will try to implement. But still, NO_GUI should ban the GUI tools from 
> being built and installed, because user can have the Tcl/Tk available, 
> but have no intention to use the git GUI. Am I right?

I am not quite certain if I agree. With a similar reasoning, you could 
introduce a flag to prevent pull-request from being installed, and 
git-tag, or other rarely used functions. Is it so bad to have gitk and 
git-gui installed? I mean, you are likely to just try them (and possibly 
like them!) at some stage, because the graphical representation is so much 
clearer than what _any_ text representation can do.

Ciao,
Dscho

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Johannes, good day.

Wed, Mar 21, 2007 at 12:16:40PM +0100, Johannes Schindelin wrote:
> > Will try to implement. But still, NO_GUI should ban the GUI tools from 
> > being built and installed, because user can have the Tcl/Tk available, 
> > but have no intention to use the git GUI. Am I right?
> 
> I am not quite certain if I agree. With a similar reasoning, you could 
> introduce a flag to prevent pull-request from being installed, and 
> git-tag, or other rarely used functions. Is it so bad to have gitk and 
> git-gui installed?

I am happening to develop on some machines on which I have no
X-Windows or any GUI providers at all, so I prefer not to have the
Tcl/Tk dependency at all. Once again, the patch was originally done
for the FreeBSD where the port system installs the dependencies
automatically. And I do not need the Tcl/Tk on some machines: imagine
the server that uses Git for its configuration tracking. It is
server, there is absolutely no need for any GUIs there. And one of
my developing machines has no X-Windows, so Tcl/Tk is again useless.
This is the reasons for the NO_GUI knob. As it is turned off by
default it does not breaks the expectations of the users who are
used to the git GUI tools.

> I mean, you are likely to just try them (and possibly 
> like them!) at some stage, because the graphical representation is so much 
> clearer than what _any_ text representation can do.

Yes, sometimes I use the graphical representation. But there are
use-cases when no GUI is needed at all.
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Johannes Schindelin]

Hi,

On Wed, 21 Mar 2007, Eygene Ryabinkin wrote:

> Wed, Mar 21, 2007 at 12:16:40PM +0100, Johannes Schindelin wrote:
> > > Will try to implement. But still, NO_GUI should ban the GUI tools from 
> > > being built and installed, because user can have the Tcl/Tk available, 
> > > but have no intention to use the git GUI. Am I right?
> > 
> > I am not quite certain if I agree. With a similar reasoning, you could 
> > introduce a flag to prevent pull-request from being installed, and 
> > git-tag, or other rarely used functions. Is it so bad to have gitk and 
> > git-gui installed?
> 
> I am happening to develop on some machines on which I have no
> X-Windows or any GUI providers at all, so I prefer not to have the
> Tcl/Tk dependency at all.

My point (and I think it's the same point as Jakub's) is that NO_GUI is a 
misnomer. It should be NO_TCL, since the only reason you state to skip 
installation of these parts is that they depend on X11, which is not 
installed on the machine.

If you don't do something, it is often interesting to state why: if you 
don't install something to prevent a dependency you don't want to have, it 
is different from saying that you do not want to have a GUI, _even if_ the 
dependency is there already.

Conclusion: I am in favour of NO_TCL, but not of NO_GUI.

Ciao,
Dscho

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Paolo Bonzini]

>> I am happening to develop on some machines on which I have no
>> X-Windows or any GUI providers at all, so I prefer not to have the
>> Tcl/Tk dependency at all.
> 
> If you don't do something, it is often interesting to state why: if you 
> don't install something to prevent a dependency you don't want to have, it 
> is different from saying that you do not want to have a GUI, _even if_ the 
> dependency is there already.

I read his message as "these are useless for me, so why introduce a
useless dependency?"  The "effect" is to have no Tcl dependence,
but the original reason is to have no GUI.

So, "If you don't do something, it is often interesting to state why".
Why no TCL (in git)?  Because no X11 (on the machine).

Paolo

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Paolo, good day.

Wed, Mar 21, 2007 at 03:38:53PM +0100, Paolo Bonzini wrote:
> >> I am happening to develop on some machines on which I have no
> >> X-Windows or any GUI providers at all, so I prefer not to have the
> >> Tcl/Tk dependency at all.
> > 
> > If you don't do something, it is often interesting to state why: if you 
> > don't install something to prevent a dependency you don't want to have, it 
> > is different from saying that you do not want to have a GUI, _even if_ the 
> > dependency is there already.
> 
> I read his message as "these are useless for me, so why introduce a
> useless dependency?"  The "effect" is to have no Tcl dependence,
> but the original reason is to have no GUI.

Yep, you're right. That was I meant originally. How do you feel
about NO_GUI or NO_TCL names?
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Paolo Bonzini]

>> I read his message as "these are useless for me, so why introduce a
>> useless dependency?"  The "effect" is to have no Tcl dependence,
>> but the original reason is to have no GUI.
> 
> Yep, you're right. That was I meant originally. How do you feel
> about NO_GUI or NO_TCL names?

I agree with you. ;-)

Paolo

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Alex Riesen]

On 3/21/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> Wed, Mar 21, 2007 at 03:38:53PM +0100, Paolo Bonzini wrote:
> > >> I am happening to develop on some machines on which I have no
> > >> X-Windows or any GUI providers at all, so I prefer not to have the
> > >> Tcl/Tk dependency at all.
> > >
> > > If you don't do something, it is often interesting to state why: if you
> > > don't install something to prevent a dependency you don't want to have, it
> > > is different from saying that you do not want to have a GUI, _even if_ the
> > > dependency is there already.
> >
> > I read his message as "these are useless for me, so why introduce a
> > useless dependency?"  The "effect" is to have no Tcl dependence,
> > but the original reason is to have no GUI.
>
> Yep, you're right. That was I meant originally. How do you feel
> about NO_GUI or NO_TCL names?

That'd be NO_TCL_TK. TCL has nothing to do with graphics.
And you have one more supporter for NO_GUI (my server has no
usable graphics, will never have and runs cron jobs with git in them).

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Jakub Narebski]

On Wed, Mar 21, 2007, Alex Riesen wrote:
> On 3/21/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:

>> Yep, you're right. That was I meant originally. How do you feel
>> about NO_GUI or NO_TCL names?
> 
> That'd be NO_TCL_TK. TCL has nothing to do with graphics.
> And you have one more supporter for NO_GUI (my server has no
> usable graphics, will never have and runs cron jobs with git in them).

Actually the value for NO_TCL_TK (or NO_TCLTK) can be detected 
automatically by ./configure in similar way that we used to detect the 
existence of Python in the old days when there were core code which 
written in Python. Perhaps we should provide way to override 
autodetection _and_ provide path to 'wish' executable with the 
--with-wish=PATH or --with-tcltk=PATH to ./configure script (similar to 
how Python dependency was handled, and how paths to shell and Perl are 
handled).

The NO_GUI is another issue, if to be configured by ./configure script, 
then only as an user option, not something to be autodetected: either 
--without-gui (treat gui as package, although usually it is about 
package to be used, not package provided) or --disable-gui (treat gui 
as feature of project).

The fact that all GUI that comes with git repository is in Tcl/Tk 
slightly clouds this issue. As do the fact that those affect only 
installation stage, and not build (well, with the exception of 
WISH_PATH / TCLTK_PATH).

-- 
Jakub Narebski
Poland

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Johannes,

Wed, Mar 21, 2007 at 03:25:32PM +0100, Johannes Schindelin wrote:
> > I am happening to develop on some machines on which I have no
> > X-Windows or any GUI providers at all, so I prefer not to have the
> > Tcl/Tk dependency at all.
> 
> My point (and I think it's the same point as Jakub's) is that NO_GUI is a 
> misnomer. It should be NO_TCL, since the only reason you state to skip 
> installation of these parts is that they depend on X11, which is not 
> installed on the machine.

Can't speak for Jakub, but it seems to me that he just suggested
that the configure should look if the Tcl/Tk is available and refuse
to install the GUI tools if there is no Tcl/Tk. Though only Jakub
can tell for sure.

> If you don't do something, it is often interesting to state why: if you 
> don't install something to prevent a dependency you don't want to have, it 
> is different from saying that you do not want to have a GUI, _even if_ the 
> dependency is there already.

But I am saying that I do not want the GUI tools installed because
I do not need GUI at all. And if the GUI will be rewritten to the
Qt (oh, my, don't do that) I will still have no need of it.

> Conclusion: I am in favour of NO_TCL, but not of NO_GUI.

I am not against such renaming as long as the TCL will be used
for the GUI part of git. Should I file a patch for the NO_GUI ->
NO_TCL change? What do others think about the knob name?
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Johannes Schindelin]

Hi,

I will make my point very clear now:

People like you compile the source not so much to develop _in_ Git, but 
_with_ Git. So, you are likely to "./configure && make install".

For something like "./configure && make install", it makes tons of sense 
to check which dependencies are there, and which are not. Then, depending 
if it is possible to compile (or install) only parts of Git, because some 
dependencies are not met, ./configure can figure out what flags to set.

Now, if you have no X11 installed (and consequently no Tk), it is very 
easy for ./configure to find out what to do.

However, if you state that you want to have no gui (and you state it as 
such), you are not caring if X11 is installed or not! And ./configure 
cannot figure out what you want, based on what it finds, since it cannot 
read your mind. At least that is what I expect.

Now, enough talk, if my point is still not clear enough, I'll just let it 
be.

Ciao,
Dscho

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Johannes,

Wed, Mar 21, 2007 at 04:35:03PM +0100, Johannes Schindelin wrote:
> I will make my point very clear now:
> 
> People like you compile the source not so much to develop _in_ Git, but 
> _with_ Git. So, you are likely to "./configure && make install".
> 
> For something like "./configure && make install", it makes tons of sense 
> to check which dependencies are there, and which are not. Then, depending 
> if it is possible to compile (or install) only parts of Git, because some 
> dependencies are not met, ./configure can figure out what flags to set.

Yes, I see your point. But up to date (with 1.5.0.3 tarball) I saw no
stock configure and thus used the 'make && make install' sequence with
the 'prefix' set to some good location. Reading the INSTALL file
I've found that the
'make configure && ./configure --prefix=<blah> && make && make install'
will do the trick too. I just was unaware of it since I was using the
first sequence. And thus I happened to do 'NO_GUI=yes make && make install'.

> Now, if you have no X11 installed (and consequently no Tk), it is very 
> easy for ./configure to find out what to do.
> 
> However, if you state that you want to have no gui (and you state it as 
> such), you are not caring if X11 is installed or not! And ./configure 
> cannot figure out what you want, based on what it finds, since it cannot 
> read your mind. At least that is what I expect.

OK. So configure needs the detection of the Tcl/Tk and the --disable-gui
option. And the Makefile can be modified to get the additional
NO_TCL_TK option that will help configure to inform the Makefile
that no Tcl/Tk is here.

Technically, the checks in Makefile will look as 'ifndef NO_GUI && NO_TCL_TK'
instead of 'ifndef NO_GUI'. Later they can diverge as the software will
evolve.

Are people happy with such plan?
-- 
Eygene

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Technically, the checks in Makefile will look as 'ifndef NO_GUI && NO_TCL_TK'
> instead of 'ifndef NO_GUI'. Later they can diverge as the software will
> evolve.
>
> Are people happy with such plan?

Maybe later you might even want to view the graphical history
from the server displaying on remote X, who knows?

We have NO_CURL and such because lack of the necessary libraries
and headers prevent your build from completing, but in the case
of git-gui and gitk, they are just scripts and you would not
have any trouble in building.  I do not know if adding more
conditional to Makefile in order to skip them is worth it.

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Eygene Ryabinkin]

Junio, good day.

Wed, Mar 21, 2007 at 09:17:21AM -0700, Junio C Hamano wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
> 
> > Technically, the checks in Makefile will look as 'ifndef NO_GUI && NO_TCL_TK'
> > instead of 'ifndef NO_GUI'. Later they can diverge as the software will
> > evolve.
> >
> > Are people happy with such plan?
> 
> Maybe later you might even want to view the graphical history
> from the server displaying on remote X, who knows?

Not sure: I don't like X on the servers ;)) But others can...

> We have NO_CURL and such because lack of the necessary libraries
> and headers prevent your build from completing, but in the case
> of git-gui and gitk, they are just scripts and you would not
> have any trouble in building.  I do not know if adding more
> conditional to Makefile in order to skip them is worth it.

OK, I reworked the patch following the suggestion of Jakub and Johannes:
now configure has the built-in detection of the Tcl/Tk binary and has
the --with-tcltk/--without-tcltk options. To implement this I still
need the NO_TCLTK knob in the Makefile. Moreover, the configure's
switch --with-tcltk=/path/to/binary works as expected: the location
of the Tcl/Tk interpreter will be rewritten in the gitk and git-gui.
The patch follows on this thread.

And regarding the building troubles and the additional knob in the
Makefile: the trouble is in the packaging process. For example, RPM
or FreeBSD ports are looking at what is really installed, so if
user do not want the Tcl/Tk part, then no package parts that depend
on it should be installed. And the bare 'make install' installs all
things. Sure, you can make 'make install && rm -f <not needed
files>', but it is always a pain for the package builders to get
the idea about the precise file list. So I just wanted to integrate
the desired behaviour in the mainstream Git to make packager's life
a bit easy. May be by the cost of making developer's life a bit
harder: he should watch for the NO_TCLTK in his Makefiles.
-- 
Eygene

[PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Eygene Ryabinkin]

The following make options were added:
- NO_TCLTK: disables building and installation of the git GUI part
that depends on the Tcl/Tk.
- WITH_P4IMPORT: enables the installation of the Perforce import
script.

Configure's options --with-tcltk and --without-tcltk were added and
configure script teached to search for the Tcl/Tk interpreter.
The GUI part will not be installed if system lacks Tcl/Tk binary.

Internal make option TCLTK was added: it governs the location of
the Tcl/Tk interpreter, so user can specify its own binary location
either with './configure --with-tcltk=/path/to/binary' or
'TCLTK=/path/to/binary make'.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile         |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 config.mak.in    |    1 +
 configure.ac     |   19 ++++++++++++++++++
 git-gui/Makefile |    1 +
 4 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 51c1fed..3cccf79 100644
--- a/Makefile
+++ b/Makefile
@@ -110,6 +110,13 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define NO_TCLTK if you do not want Tcl/Tk GUI.
+#
+# The TCLTK variable governs the location of the Tck/Tk interpreter.
+# If not set it defaults to the bare 'wish'.
+#
+# Define WITH_P4IMPORT to build and install Python git-p4import script.
+#
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -159,6 +166,7 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
+TCLTK ?= wish
 
 # sparse is architecture-neutral, which means that we need to tell it
 # explicitly what architecture to check for. Fix this up for yours..
@@ -196,9 +204,20 @@ SCRIPT_PERL = \
 	git-svnimport.perl git-cvsexportcommit.perl \
 	git-send-email.perl git-svn.perl
 
+SCRIPT_PYTHON = \
+	git-p4import.py
+
+ifdef WITH_P4IMPORT
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
 	  git-status git-instaweb
+else
+SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
+	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+	  git-status git-instaweb
+endif
+
 
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS = \
@@ -241,6 +260,9 @@ endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl
 endif
+ifndef PYTHON_PATH
+	PYTHON_PATH = /usr/local/bin/python
+endif
 
 export PERL_PATH
 
@@ -608,6 +630,10 @@ ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
 
+ifeq ($(TCLTK),)
+NO_TCLTK=YesPlease
+endif
+
 QUIET_SUBDIR0  = $(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -646,6 +672,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
@@ -667,7 +694,9 @@ ifneq (,$X)
 endif
 
 all::
+ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
+endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
@@ -699,6 +728,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
+$(patsubst %.py,%,$(SCRIPT_PYTHON)) : % : %.py
+	rm -f $@ $@+
+	sed -e '1s|#!.*/python|#!$(PYTHON_PATH_SQ)|' \
+	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    $@.py >$@+
+	chmod +x $@+
+	mv $@+ $@
+
 perl/perl.mak: GIT-CFLAGS
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
 
@@ -892,10 +930,16 @@ install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+ifndef NO_TCLTK
+	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK)"' "$$0"|' gitk && rm -f gitk.bak
+	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' install
-	$(MAKE) -C git-gui install
+ifndef NO_TCLTK
+	$(MAKE) -C git-gui TCLTK='$(TCLTK)' install
+endif
 	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
@@ -929,11 +973,17 @@ dist: git.spec git-archive
 	@mkdir -p $(GIT_TARNAME)
 	@cp git.spec $(GIT_TARNAME)
 	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
+ifndef NO_TCLTK
 	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
 	$(TAR) rf $(GIT_TARNAME).tar \
 		$(GIT_TARNAME)/git.spec \
 		$(GIT_TARNAME)/version \
 		$(GIT_TARNAME)/git-gui/version
+else
+	$(TAR) rf $(GIT_TARNAME).tar \
+		$(GIT_TARNAME)/git.spec \
+		$(GIT_TARNAME)/version
+endif
 	@rm -rf $(GIT_TARNAME)
 	gzip -f -9 $(GIT_TARNAME).tar
 
@@ -974,7 +1024,9 @@ clean:
 	rm -f gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
 	$(MAKE) -C perl clean
+ifndef NO_TCLTK
 	$(MAKE) -C git-gui clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS
diff --git a/config.mak.in b/config.mak.in
index 9a57840..8e441dd 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -6,6 +6,7 @@ CFLAGS = @CFLAGS@
 AR = @AR@
 TAR = @TAR@
 #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
+TCLTK = @TCLTK@
 
 prefix = @prefix@
 exec_prefix = @exec_prefix@
diff --git a/configure.ac b/configure.ac
index 3a8e778..a95dbfb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,12 @@ GIT_ARG_SET_PATH(shell)
 # Define PERL_PATH to provide path to Perl.
 GIT_ARG_SET_PATH(perl)
 #
+# Declare the with-tcltk/without-tcltk options.
+AC_ARG_WITH(tcltk,
+AS_HELP_STRING([--with-tcltk],[use Tcl/Tk GUI (default is YES)])
+AS_HELP_STRING([],[ARG is the full path to the Tcl/Tk interpreter]),\
+GIT_PARSE_WITH(tcltk))
+#
 
 
 ## Checks for programs.
@@ -84,6 +90,19 @@ AC_PROG_CC([cc gcc])
 #AC_PROG_INSTALL		# needs install-sh or install.sh in sources
 AC_CHECK_TOOL(AR, ar, :)
 AC_CHECK_PROGS(TAR, [gtar tar])
+# TCLTK will be set to some value if we want Tcl/Tk
+# or will be empty otherwise.
+if test -z "$NO_TCLTK"; then
+  if test "$with_tcltk" = "yes" -o "$with_tcltk" = ""; then
+    AC_CHECK_PROGS(TCLTK, [wish], )
+  elif ! test -x "$with_tcltk"; then
+    AC_MSG_ERROR([Tcl/Tk interpreter was not found in $with_tcltk])
+  else
+    AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])
+    TCLTK="$with_tcltk"
+    AC_SUBST(TCLTK)
+  fi
+fi
 
 ## Checks for libraries.
 AC_MSG_NOTICE([CHECKS for libraries])
diff --git a/git-gui/Makefile b/git-gui/Makefile
index b82789e..09c28ed 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -55,6 +55,7 @@ all:: $(ALL_PROGRAMS)
 
 install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK)"' "$$0"|' git-gui && rm git-gui.bak
 	$(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' ;)
 
-- 
1.5.0.3-dirty

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Junio C Hamano]

Please do not mix P4IMPORT and TCLTK issues in the same patch.
As I am still not convinced about what to do with NO_TCLTK, this
forces people who might want to see P4IMPORT to wait longer than
necessary.

[PATCH] Add the WITH_P4IMPORT knob to the Makefile.

[Eygene Ryabinkin]

WITH_P4IMPORT: enables the installation of the Perforce import
script.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 51c1fed..a294ec8 100644
--- a/Makefile
+++ b/Makefile
@@ -110,6 +110,8 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define WITH_P4IMPORT to build and install Python git-p4import script.
+#
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -196,9 +198,20 @@ SCRIPT_PERL = \
 	git-svnimport.perl git-cvsexportcommit.perl \
 	git-send-email.perl git-svn.perl
 
+SCRIPT_PYTHON = \
+	git-p4import.py
+
+ifdef WITH_P4IMPORT
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
 	  git-status git-instaweb
+else
+SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
+	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+	  git-status git-instaweb
+endif
+
 
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS = \
@@ -241,6 +254,9 @@ endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl
 endif
+ifndef PYTHON_PATH
+	PYTHON_PATH = /usr/local/bin/python
+endif
 
 export PERL_PATH
 
@@ -646,6 +662,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
@@ -699,6 +716,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
+$(patsubst %.py,%,$(SCRIPT_PYTHON)) : % : %.py
+	rm -f $@ $@+
+	sed -e '1s|#!.*/python|#!$(PYTHON_PATH_SQ)|' \
+	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    $@.py >$@+
+	chmod +x $@+
+	mv $@+ $@
+
 perl/perl.mak: GIT-CFLAGS
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
 
-- 
1.5.0.3-dirty

Re: [PATCH] Add the WITH_P4IMPORT knob to the Makefile.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> WITH_P4IMPORT: enables the installation of the Perforce import
> script.
>
> Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>

Thanks.  I wonder if we need to update git.spec.in file if we
were to take this patch.

Re: [PATCH] Add the WITH_P4IMPORT knob to the Makefile.

[Eygene Ryabinkin]

Tue, Mar 27, 2007 at 03:54:16AM -0700, Junio C Hamano wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
> 
> > WITH_P4IMPORT: enables the installation of the Perforce import
> > script.
> >
> > Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
> 
> Thanks.

You're welcome ;))

> I wonder if we need to update git.spec.in file if we
> were to take this patch.

Created a quick patch, but have no Linux host with autoconf >= 2.59
at hand, so can not test. The patch follows.
-- 
Eygene

[PATCH] Added git-p4 package to the list of git RPMs.

[Eygene Ryabinkin]

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 git.spec.in |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index 46aee88..8c79a79 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -50,6 +50,13 @@ Requires:       git-core = %{version}-%{release}, tla
 %description arch
 Git tools for importing Arch repositories.
 
+%package p4
+Summary:        Git tools for importing Perforce repositories
+Group:          Development/Tools
+Requires:       git-core = %{version}-%{release}, python
+%description p4
+Git tools for importing Perforce repositories.
+
 %package email
 Summary:        Git tools for sending email
 Group:          Development/Tools
@@ -86,22 +93,22 @@ Perl interface to Git
 
 %build
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" WITH_OWN_SUBPROCESS_PY=YesPlease \
-     prefix=%{_prefix} all %{!?_without_docs: doc}
+     WITH_P4IMPORT=YesPlease prefix=%{_prefix} all %{!?_without_docs: doc}
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" DESTDIR=$RPM_BUILD_ROOT \
-     WITH_OWN_SUBPROCESS_PY=YesPlease \
+     WITH_OWN_SUBPROCESS_PY=YesPlease WITH_P4IMPORT=YesPlease \
      prefix=%{_prefix} mandir=%{_mandir} INSTALLDIRS=vendor \
      install %{!?_without_docs: install-doc}
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name '*.bs' -empty -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name perllocal.pod -exec rm -f {} ';'
 
-(find $RPM_BUILD_ROOT%{_bindir} -type f | grep -vE "archimport|svn|cvs|email|gitk|git-gui|git-citool" | sed -e s@^$RPM_BUILD_ROOT@@)               > bin-man-doc-files
+(find $RPM_BUILD_ROOT%{_bindir} -type f | grep -vE "p4import|archimport|svn|cvs|email|gitk|git-gui|git-citool" | sed -e s@^$RPM_BUILD_ROOT@@)               > bin-man-doc-files
 (find $RPM_BUILD_ROOT%{perl_vendorlib} -type f | sed -e s@^$RPM_BUILD_ROOT@@) >> perl-files
 %if %{!?_without_docs:1}0
-(find $RPM_BUILD_ROOT%{_mandir} $RPM_BUILD_ROOT/Documentation -type f | grep -vE "archimport|svn|git-cvs|email|gitk|git-gui|git-citool" | sed -e s@^$RPM_BUILD_ROOT@@ -e 's/$/*/' ) >> bin-man-doc-files
+(find $RPM_BUILD_ROOT%{_mandir} $RPM_BUILD_ROOT/Documentation -type f | grep -vE "p4import|archimport|svn|git-cvs|email|gitk|git-gui|git-citool" | sed -e s@^$RPM_BUILD_ROOT@@ -e 's/$/*/' ) >> bin-man-doc-files
 %else
 rm -rf $RPM_BUILD_ROOT%{_mandir}
 %endif
@@ -133,6 +140,13 @@ rm -rf $RPM_BUILD_ROOT
 %{!?_without_docs: %{_mandir}/man1/git-archimport.1*}
 %{!?_without_docs: %doc Documentation/git-archimport.html }
 
+%files p4
+%defattr(-,root,root)
+%doc Documentation/git-p4import.txt
+%{_bindir}/git-p4import
+%{!?_without_docs: %{_mandir}/man1/git-p4import.1*}
+%{!?_without_docs: %doc Documentation/git-p4import.html }
+
 %files email
 %defattr(-,root,root)
 %doc Documentation/*email*.txt
@@ -167,6 +181,9 @@ rm -rf $RPM_BUILD_ROOT
 %{!?_without_docs: %doc Documentation/*.html }
 
 %changelog
+* Tue Mar 27 2007 Eygene Ryabinkin <rea-git@codelabs.ru>
+- Added the git-p4 package: Perforce import stuff.
+
 * Mon Feb 13 2007 Nicolas Pitre <nico@cam.org>
 - Update core package description (Git isn't as stupid as it used to be)
 
-- 
1.5.0.3-dirty

[PATCH] Remove unused WITH_OWN_SUBPROCESS_PY from RPM spec

[Brian Gernhardt]

We don't have a copy of subprocess.py anymore, so we removed that
option from the Makefile.  Let's not leave that cruft around the RPM
spec file either.
---

This applies on top of "[PATCH] Added git-p4 package to the list of git
RPMs.", which is what made me notice that it was there.  Untested, but
simply removing a completely unused option from the make command line
shouldn't cause any problems, right?  Right?

 git.spec.in |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index e469f21..4bf7a8f 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -92,15 +92,14 @@ Perl interface to Git
 %setup -q
 
 %build
-make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" WITH_OWN_SUBPROCESS_PY=YesPlease \
-     WITH_P4IMPORT=YesPlease prefix=%{_prefix} all %{!?_without_docs: doc}
+make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" WITH_P4IMPORT=YesPlease \
+     prefix=%{_prefix} all %{!?_without_docs: doc}
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" DESTDIR=$RPM_BUILD_ROOT \
-     WITH_OWN_SUBPROCESS_PY=YesPlease WITH_P4IMPORT=YesPlease \
-     prefix=%{_prefix} mandir=%{_mandir} INSTALLDIRS=vendor \
-     install %{!?_without_docs: install-doc}
+     WITH_P4IMPORT=YesPlease prefix=%{_prefix} mandir=%{_mandir} \
+     INSTALLDIRS=vendor install %{!?_without_docs: install-doc}
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name '*.bs' -empty -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name perllocal.pod -exec rm -f {} ';'
-- 
1.5.1.rc2

Re: [PATCH] Added git-p4 package to the list of git RPMs.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> diff --git a/git.spec.in b/git.spec.in
> index 46aee88..8c79a79 100644
> --- a/git.spec.in
> +++ b/git.spec.in
> @@ -50,6 +50,13 @@ Requires:       git-core = %{version}-%{release}, tla
>  %description arch
>  Git tools for importing Arch repositories.
>  
> +%package p4
> +Summary:        Git tools for importing Perforce repositories
> +Group:          Development/Tools
> +Requires:       git-core = %{version}-%{release}, python
> +%description p4
> +Git tools for importing Perforce repositories.
> +
>...

Thanks. I'll also add git-p4 here.

diff --git a/git.spec.in b/git.spec.in
index 1d3934b..f0746ed 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -12,7 +12,7 @@ URL: 		http://kernel.org/pub/software/scm/git/
 Source: 	http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
 BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-Requires:	git-core, git-svn, git-cvs, git-arch, git-email, gitk, git-gui, perl-Git
+Requires:	git-core, git-svn, git-cvs, git-arch, git-email, gitk, git-gui, git-p4, perl-Git
 
 %description
 Git is a fast, scalable, distributed revision control system with an

Re: [PATCH] Added git-p4 package to the list of git RPMs.

[Eygene Ryabinkin]

Junio, good day.

Wed, Apr 04, 2007 at 11:30:55AM -0700, Junio C Hamano wrote:
> Thanks. I'll also add git-p4 here.

Thank you.
-- 
Eygene

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Jakub Narebski]

Thanks a lot for the patch, but...

Eygene Ryabinkin wrote:

> Internal make option TCLTK was added: it governs the location of
> the Tcl/Tk interpreter, so user can specify its own binary location
> either with './configure --with-tcltk=/path/to/binary' or
> 'TCLTK=/path/to/binary make'.

...shouldn't it be TCLTK_PATH?
-- 
Jakub Narebski
Poland

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Thanks a lot for the patch, but...
>
> Eygene Ryabinkin wrote:
>
>> Internal make option TCLTK was added: it governs the location of
>> the Tcl/Tk interpreter, so user can specify its own binary location
>> either with './configure --with-tcltk=/path/to/binary' or
>> 'TCLTK=/path/to/binary make'.
>
> ...shouldn't it be TCLTK_PATH?

Thanks for sanity checking.  That means that the absense of
tcltk would make it impossible to munge the scripts to point at
the wish binary, so makes the NO_TCLTK stuff easier to swallow.

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Eygene Ryabinkin]

Jakub, Junio,

Mon, Mar 26, 2007 at 01:36:49AM -0700, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Thanks a lot for the patch, but...
> >
> > Eygene Ryabinkin wrote:
> >
> >> Internal make option TCLTK was added: it governs the location of
> >> the Tcl/Tk interpreter, so user can specify its own binary location
> >> either with './configure --with-tcltk=/path/to/binary' or
> >> 'TCLTK=/path/to/binary make'.
> >
> > ...shouldn't it be TCLTK_PATH?

Yes, probably is should be TCLTK_PATH.

> Thanks for sanity checking.  That means that the absense of
> tcltk would make it impossible to munge the scripts to point at
> the wish binary, so makes the NO_TCLTK stuff easier to swallow.

Sorry, did not get the point. The TCLTK is initialized to the 'wish'
by 'TCLTK ?= wish', so TCLTK will always be here and initialized
to the wish by-default.
-- 
Eygene

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Mon, Mar 26, 2007 at 01:36:49AM -0700, Junio C Hamano wrote:
> ...
>> Thanks for sanity checking.  That means that the absense of
>> tcltk would make it impossible to munge the scripts to point at
>> the wish binary, so makes the NO_TCLTK stuff easier to swallow.
>
> Sorry, did not get the point. The TCLTK is initialized to the 'wish'
> by 'TCLTK ?= wish', so TCLTK will always be here and initialized
> to the wish by-default.

Earlier I said I did not see a reason for not building wish
applications on a build system that lack them.  I am stating
that you could argue that your rewriting the path to wish is a
good reason (I would say it is half-good, as you can still tell
the build procedure where wish will be on the deployed system
without having it on your build system) for not building wish
applications in a build that lacks wish installation.

Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.

[Eygene Ryabinkin]

Junio, good day.

Mon, Mar 26, 2007 at 09:12:17PM -0700, Junio C Hamano wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
> 
> > Mon, Mar 26, 2007 at 01:36:49AM -0700, Junio C Hamano wrote:
> > ...
> >> Thanks for sanity checking.  That means that the absense of
> >> tcltk would make it impossible to munge the scripts to point at
> >> the wish binary, so makes the NO_TCLTK stuff easier to swallow.
> >
> > Sorry, did not get the point. The TCLTK is initialized to the 'wish'
> > by 'TCLTK ?= wish', so TCLTK will always be here and initialized
> > to the wish by-default.
> 
> Earlier I said I did not see a reason for not building wish
> applications on a build system that lack them.  I am stating
> that you could argue that your rewriting the path to wish is a
> good reason (I would say it is half-good, as you can still tell
> the build procedure where wish will be on the deployed system
> without having it on your build system) for not building wish
> applications in a build that lacks wish installation.

OK, so, probably, I should modify the behaviour of the --with-tcltk
and configure to look for the Tcl/Tk interpreter _only_ if
--with-tcltk[=PATH] was given and to leave the things unmodified
in the case of absence of that option. But still, --without-tcltk
will disable Tcl/Tk dependant parts. Will people be happy with such
behaviour?
-- 
Eygene

[PATCH] Added configure options --with-tcltk/--without-tcltk.

[Eygene Ryabinkin]

Configure's options --with-tcltk and --without-tcltk were added and
configure script teached to search for the Tcl/Tk interpreter.

The default behaviour to install Tcl/Tk dependant parts is left
intact: Tcl/Tk detection will be enabled only if --with-tcltk option
is given to configure.

Makefiles got two external options:
- TCLTK_PATH: the path to the Tcl/Tk interpreter.
- NO_TCLCK: disables the installation of Tcl/Tk dependend parts.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile         |   30 ++++++++++++++++++++++++++++--
 config.mak.in    |    1 +
 configure.ac     |   26 ++++++++++++++++++++++++++
 git-gui/Makefile |    3 +++
 4 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a294ec8..06b6c6b 100644
--- a/Makefile
+++ b/Makefile
@@ -112,6 +112,11 @@ all::
 #
 # Define WITH_P4IMPORT to build and install Python git-p4import script.
 #
+# Define NO_TCLTK if you do not want Tcl/Tk GUI.
+#
+# The TCLTK_PATH variable governs the location of the Tck/Tk interpreter.
+# If not set it defaults to the bare 'wish'.
+#
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -161,6 +166,7 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
+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..
@@ -624,6 +630,10 @@ ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
 
+ifeq ($(TCLTK_PATH),)
+NO_TCLTK=YesPlease
+endif
+
 QUIET_SUBDIR0  = $(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -684,7 +694,9 @@ ifneq (,$X)
 endif
 
 all::
+ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
+endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
@@ -918,10 +930,16 @@ install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+ifndef NO_TCLTK
+	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK_PATH)"' "$$0"|' gitk && rm -f gitk.bak
+	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' install
-	$(MAKE) -C git-gui install
+ifndef NO_TCLTK
+	$(MAKE) -C git-gui TCLTK_PATH='$(TCLTK_PATH)' install
+endif
 	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
@@ -955,11 +973,17 @@ dist: git.spec git-archive
 	@mkdir -p $(GIT_TARNAME)
 	@cp git.spec $(GIT_TARNAME)
 	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
+ifndef NO_TCLTK
 	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
 	$(TAR) rf $(GIT_TARNAME).tar \
 		$(GIT_TARNAME)/git.spec \
 		$(GIT_TARNAME)/version \
 		$(GIT_TARNAME)/git-gui/version
+else
+	$(TAR) rf $(GIT_TARNAME).tar \
+		$(GIT_TARNAME)/git.spec \
+		$(GIT_TARNAME)/version
+endif
 	@rm -rf $(GIT_TARNAME)
 	gzip -f -9 $(GIT_TARNAME).tar
 
@@ -1000,7 +1024,9 @@ clean:
 	rm -f gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
 	$(MAKE) -C perl clean
+ifndef NO_TCLTK
 	$(MAKE) -C git-gui clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS
diff --git a/config.mak.in b/config.mak.in
index 9a57840..eb9d7a5 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -6,6 +6,7 @@ CFLAGS = @CFLAGS@
 AR = @AR@
 TAR = @TAR@
 #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
+TCLTK_PATH = @TCLTK_PATH@
 
 prefix = @prefix@
 exec_prefix = @exec_prefix@
diff --git a/configure.ac b/configure.ac
index 3a8e778..43a6769 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,14 @@ GIT_ARG_SET_PATH(shell)
 # Define PERL_PATH to provide path to Perl.
 GIT_ARG_SET_PATH(perl)
 #
+# Declare the with-tcltk/without-tcltk options.
+AC_ARG_WITH(tcltk,
+AS_HELP_STRING([--with-tcltk],[use Tcl/Tk GUI (default is YES)])
+AS_HELP_STRING([],[ARG is the full path to the Tcl/Tk interpreter.])
+AS_HELP_STRING([],[Bare --with-tcltk will make the GUI part only if])
+AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a system.]),\
+GIT_PARSE_WITH(tcltk))
+#
 
 
 ## Checks for programs.
@@ -84,6 +92,24 @@ AC_PROG_CC([cc gcc])
 #AC_PROG_INSTALL		# needs install-sh or install.sh in sources
 AC_CHECK_TOOL(AR, ar, :)
 AC_CHECK_PROGS(TAR, [gtar tar])
+# TCLTK_PATH will be set to some value if we want Tcl/Tk
+# or will be empty otherwise.
+if test -z "$NO_TCLTK"; then
+  if test "$with_tcltk" = ""; then
+  # No Tcl/Tk switches given. Do not check for Tcl/Tk, use bare 'wish'.
+    TCLTK_PATH=wish
+    AC_SUBST(TCLTK_PATH)
+  elif test "$with_tcltk" = "yes"; then
+  # Tcl/Tk check requested.
+    AC_CHECK_PROGS(TCLTK_PATH, [wish], )
+  elif ! test -x "$with_tcltk"; then
+    AC_MSG_ERROR([Tcl/Tk interpreter was not found in $with_tcltk])
+  else
+    AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])
+    TCLTK_PATH="$with_tcltk"
+    AC_SUBST(TCLTK_PATH)
+  fi
+fi
 
 ## Checks for libraries.
 AC_MSG_NOTICE([CHECKS for libraries])
diff --git a/git-gui/Makefile b/git-gui/Makefile
index b82789e..2316b24 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -28,6 +28,8 @@ ifndef V
 	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
 endif
 
+TCLTK_PATH ?= wish
+
 ifeq ($(findstring $(MAKEFLAGS),s),s)
 QUIET_GEN =
 QUIET_BUILT_IN =
@@ -55,6 +57,7 @@ all:: $(ALL_PROGRAMS)
 
 install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK_PATH)"' "$$0"|' git-gui && rm git-gui.bak
 	$(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' ;)
 
-- 
1.5.0.3-dirty

Re: [PATCH] Added configure options --with-tcltk/--without-tcltk.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Configure's options --with-tcltk and --without-tcltk were added and
> configure script teached to search for the Tcl/Tk interpreter.
>
> The default behaviour to install Tcl/Tk dependant parts is left
> intact: Tcl/Tk detection will be enabled only if --with-tcltk option
> is given to configure.
>
> Makefiles got two external options:
> - TCLTK_PATH: the path to the Tcl/Tk interpreter.
> - NO_TCLCK: disables the installation of Tcl/Tk dependend parts.
>
> Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>

Thanks.

> diff --git a/Makefile b/Makefile
> index a294ec8..06b6c6b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -112,6 +112,11 @@ all::
>  #
>  # Define WITH_P4IMPORT to build and install Python git-p4import script.
>  #
> +# Define NO_TCLTK if you do not want Tcl/Tk GUI.
> +#
> +# The TCLTK_PATH variable governs the location of the Tck/Tk interpreter.
> +# If not set it defaults to the bare 'wish'.
> +#
>...
> +TCLTK_PATH ?= wish
> ...  
> +ifeq ($(TCLTK_PATH),)
> +NO_TCLTK=YesPlease
> +endif
> +

This seems to contradict the log message that makes these two
options sound as if they are not dependent of each other.

> @@ -918,10 +930,16 @@ install: all
>  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
>  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
> -	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
> +	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
> +ifndef NO_TCLTK
> +	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK_PATH)"' "$$0"|' gitk && rm -f gitk.bak
> +	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
> +endif

This is a no-no. "make $args; su make $args install" should
never cause anything built by root with the second invocation of
the make command.  Don't assume you can write into the build
directory while you are running "make install" (root user can be
mapped nobody on a nfs mounted build directory, while the local
target directory is writable by it).

Also please quote $(TCLTK_PATH) like everybody else does in the
Makefile.  For that purpose, I think the way $(SCRIPT_SH) are
built using $(SHELL_PATH_SQ) can be learned from.

I suspect that the change to allow not installing gitk/git-gui
and the change to allow using specific "wish" are two
independent tasks.  You seem to have a grip on the use of
conditional in Makefile to do the former task, and I do not
think there is any need for further commenting.

For the latter task, you can probably do something like this:

	gitk-wish: gitk 
        	rm -f $@+ $@
		sed -e '3s| wish | ...' <gitk >$@+
                mv $@+ $@

	all:: gitk-wish
	install: all
        	...
                $(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk

Also you need to rebuild gitk-wish when the builder gives
different TCLTK_PATH; I suspect the easiest way is tack it to
TRACK_CFLAGS and make gitk-wish depend on GIT-CFLAGS.

Re: [PATCH] Added configure options --with-tcltk/--without-tcltk.

[Eygene Ryabinkin]

Junio, good day.

Tue, Mar 27, 2007 at 03:53:33AM -0700, Junio C Hamano wrote:
> > +ifeq ($(TCLTK_PATH),)
> > +NO_TCLTK=YesPlease
> > +endif
> > +
> 
> This seems to contradict the log message that makes these two
> options sound as if they are not dependent of each other.

OK, will add a sentence about the dependency to the log.

> 
> > @@ -918,10 +930,16 @@ install: all
> >  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
> >  	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
> >  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
> > -	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
> > +	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
> > +ifndef NO_TCLTK
> > +	sed -i .bak -e'1,3s|^exec .* "$$0"|exec '"$(TCLTK_PATH)"' "$$0"|' gitk && rm -f gitk.bak
> > +	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
> > +endif
> 
> This is a no-no. "make $args; su make $args install" should
> never cause anything built by root with the second invocation of
> the make command.  Don't assume you can write into the build
> directory while you are running "make install" (root user can be
> mapped nobody on a nfs mounted build directory, while the local
> target directory is writable by it).
> Also please quote $(TCLTK_PATH) like everybody else does in the
> Makefile.  For that purpose, I think the way $(SCRIPT_SH) are
> built using $(SHELL_PATH_SQ) can be learned from.

OK.

> I suspect that the change to allow not installing gitk/git-gui
> and the change to allow using specific "wish" are two
> independent tasks.

But then the configure will be first teached to recognise only
'--with-tcltk/--without-tcltk' and the second modification will
add '--with-tcltk=/path/to/wish', right?

> You seem to have a grip on the use of
> conditional in Makefile to do the former task, and I do not
> think there is any need for further commenting.
> 
> For the latter task, you can probably do something like this:
> 
> 	gitk-wish: gitk 
>         	rm -f $@+ $@
> 		sed -e '3s| wish | ...' <gitk >$@+
>                 mv $@+ $@
> 
> 	all:: gitk-wish
> 	install: all
>         	...
>                 $(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
> 
> Also you need to rebuild gitk-wish when the builder gives
> different TCLTK_PATH; I suspect the easiest way is tack it to
> TRACK_CFLAGS and make gitk-wish depend on GIT-CFLAGS.

OK, will try to provide the splitted patches.
-- 
Eygene

Re: [PATCH] Added configure options --with-tcltk/--without-tcltk.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

>> I suspect that the change to allow not installing gitk/git-gui
>> and the change to allow using specific "wish" are two
>> independent tasks.
>
> But then the configure will be first teached to recognise only
> '--with-tcltk/--without-tcltk' and the second modification will
> add '--with-tcltk=/path/to/wish', right?

That sounds sensible to me.

Thanks.

[PATCH] Add --with-tcltk and --without-tcltk to configure.

[Eygene Ryabinkin]

--with-tcltk enables the search of the Tcl/Tk interpreter. If no
interpreter is found then Tcl/Tk dependend parts are disabled.

--without-tcltk unconditionally disables Tcl/Tk dependent parts.

The original behaviour is not changed: bare './configure' just
installs the Tcl/Tk part doing no checks for the interpreter.

Makefile knob named NO_TCLTK was introduced. It prevents the build
and installation of the Tcl/Tk dependent parts.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile         |   31 +++++++++++++++++++++++++++++--
 config.mak.in    |    1 +
 configure.ac     |   26 ++++++++++++++++++++++++++
 git-gui/Makefile |    3 +++
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a294ec8..bfde029 100644
--- a/Makefile
+++ b/Makefile
@@ -112,6 +112,12 @@ all::
 #
 # Define WITH_P4IMPORT to build and install Python git-p4import script.
 #
+# Define NO_TCLTK if you do not want Tcl/Tk GUI.
+#
+# The TCLTK_PATH variable governs the location of the Tck/Tk interpreter.
+# If not set it defaults to the bare 'wish'. If it is set to the empty
+# string then NO_TCLTK will be forced (this is used by configure script).
+#
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -161,6 +167,7 @@ AR = ar
 TAR = tar
 INSTALL = install
 RPMBUILD = rpmbuild
+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..
@@ -624,6 +631,10 @@ ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
 
+ifeq ($(TCLTK_PATH),)
+NO_TCLTK=YesPlease
+endif
+
 QUIET_SUBDIR0  = $(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -663,6 +674,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
+TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
@@ -684,7 +696,9 @@ ifneq (,$X)
 endif
 
 all::
+ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
+endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
@@ -918,10 +932,15 @@ install: all
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) git$X gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
+ifndef NO_TCLTK
+	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' install
-	$(MAKE) -C git-gui install
+ifndef NO_TCLTK
+	$(MAKE) -C git-gui TCLTK_PATH='$(TCLTK_PATH_SQ)' install
+endif
 	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
@@ -955,11 +974,17 @@ dist: git.spec git-archive
 	@mkdir -p $(GIT_TARNAME)
 	@cp git.spec $(GIT_TARNAME)
 	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
+ifndef NO_TCLTK
 	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
 	$(TAR) rf $(GIT_TARNAME).tar \
 		$(GIT_TARNAME)/git.spec \
 		$(GIT_TARNAME)/version \
 		$(GIT_TARNAME)/git-gui/version
+else
+	$(TAR) rf $(GIT_TARNAME).tar \
+		$(GIT_TARNAME)/git.spec \
+		$(GIT_TARNAME)/version
+endif
 	@rm -rf $(GIT_TARNAME)
 	gzip -f -9 $(GIT_TARNAME).tar
 
@@ -1000,7 +1025,9 @@ clean:
 	rm -f gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
 	$(MAKE) -C perl clean
+ifndef NO_TCLTK
 	$(MAKE) -C git-gui clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS
diff --git a/config.mak.in b/config.mak.in
index 9a57840..eb9d7a5 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -6,6 +6,7 @@ CFLAGS = @CFLAGS@
 AR = @AR@
 TAR = @TAR@
 #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
+TCLTK_PATH = @TCLTK_PATH@
 
 prefix = @prefix@
 exec_prefix = @exec_prefix@
diff --git a/configure.ac b/configure.ac
index 3a8e778..43a6769 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,14 @@ GIT_ARG_SET_PATH(shell)
 # Define PERL_PATH to provide path to Perl.
 GIT_ARG_SET_PATH(perl)
 #
+# Declare the with-tcltk/without-tcltk options.
+AC_ARG_WITH(tcltk,
+AS_HELP_STRING([--with-tcltk],[use Tcl/Tk GUI (default is YES)])
+AS_HELP_STRING([],[ARG is the full path to the Tcl/Tk interpreter.])
+AS_HELP_STRING([],[Bare --with-tcltk will make the GUI part only if])
+AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a system.]),\
+GIT_PARSE_WITH(tcltk))
+#
 
 
 ## Checks for programs.
@@ -84,6 +92,24 @@ AC_PROG_CC([cc gcc])
 #AC_PROG_INSTALL		# needs install-sh or install.sh in sources
 AC_CHECK_TOOL(AR, ar, :)
 AC_CHECK_PROGS(TAR, [gtar tar])
+# TCLTK_PATH will be set to some value if we want Tcl/Tk
+# or will be empty otherwise.
+if test -z "$NO_TCLTK"; then
+  if test "$with_tcltk" = ""; then
+  # No Tcl/Tk switches given. Do not check for Tcl/Tk, use bare 'wish'.
+    TCLTK_PATH=wish
+    AC_SUBST(TCLTK_PATH)
+  elif test "$with_tcltk" = "yes"; then
+  # Tcl/Tk check requested.
+    AC_CHECK_PROGS(TCLTK_PATH, [wish], )
+  elif ! test -x "$with_tcltk"; then
+    AC_MSG_ERROR([Tcl/Tk interpreter was not found in $with_tcltk])
+  else
+    AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])
+    TCLTK_PATH="$with_tcltk"
+    AC_SUBST(TCLTK_PATH)
+  fi
+fi
 
 ## Checks for libraries.
 AC_MSG_NOTICE([CHECKS for libraries])
diff --git a/git-gui/Makefile b/git-gui/Makefile
index b82789e..733c07e 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -28,6 +28,8 @@ ifndef V
 	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
 endif
 
+TCLTK_PATH ?= wish
+
 ifeq ($(findstring $(MAKEFLAGS),s),s)
 QUIET_GEN =
 QUIET_BUILT_IN =
@@ -36,6 +38,7 @@ endif
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	$(QUIET_GEN)rm -f $@ $@+ && \
-- 
1.5.0.3-dirty

[PATCH] Added Tcl/Tk interpreter path rewriting for the GUI tools.

[Eygene Ryabinkin]

--with-tcltk=/path/to/wish sets the TCLTK_PATH variable that is
used to substitute the location of the wish interpreter in the
Tcl/Tk programs.

New tracking file, GIT-GUI-VARS, was introduced: it tracks the
location of the Tcl/Tk interpreter and activates the GUI tools
rebuild if the interpreter path was changed. The separate tracker
is better than the GIT-CFLAGS: there is no need to rebuild the whole
git if the interpreter path was changed.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 Makefile         |   31 ++++++++++++++++++++++++++++---
 git-gui/Makefile |   17 +++++++++++++++--
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index bfde029..0034666 100644
--- a/Makefile
+++ b/Makefile
@@ -690,11 +690,15 @@ export prefix gitexecdir TAR INSTALL DESTDIR SHELL_PATH template_dir
 
 ### Build rules
 
-all:: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk gitweb/gitweb.cgi
+all:: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitweb/gitweb.cgi
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), rm -f '$p';)
 endif
 
+ifndef NO_TCLTK
+all:: gitk-wish
+endif
+
 all::
 ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
@@ -705,6 +709,12 @@ endif
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
+gitk-wish: gitk GIT-GUI-VARS
+	$(QUIET_GEN)rm -f $@ $@+ && \
+	sed -e'1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' < gitk > $@+ && \
+	chmod +x $@+ && \
+	mv -f $@+ $@
+
 git$X: git.c common-cmds.h $(BUILTIN_OBJS) $(GITLIBS) GIT-CFLAGS
 	$(QUIET_LINK)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
 		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) \
@@ -892,6 +902,18 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 		echo "$$FLAGS" >GIT-CFLAGS; \
             fi
 
+### Detect Tck/Tk interpreter path changes
+ifndef NO_TCLTK
+TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
+
+GIT-GUI-VARS: .FORCE-GIT-GUI-VARS
+	@VARS='$(TRACK_VARS)'; \
+	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+		echo 1>&2 "    * new Tcl/Tk interpreter location"; \
+		echo "$$VARS" >$@; \
+            fi
+endif
+
 ### Testing rules
 
 # GNU make supports exporting all variables by "export" without parameters.
@@ -934,7 +956,7 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(INSTALL) git$X '$(DESTDIR_SQ)$(bindir_SQ)'
 ifndef NO_TCLTK
-	$(INSTALL) gitk '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
 endif
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' install
@@ -1030,10 +1052,13 @@ ifndef NO_TCLTK
 endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
-	rm -f GIT-VERSION-FILE GIT-CFLAGS
+	rm -f GIT-VERSION-FILE GIT-CFLAGS GIT-GUI-VARS
 
 .PHONY: all install clean strip
 .PHONY: .FORCE-GIT-VERSION-FILE TAGS tags .FORCE-GIT-CFLAGS
+ifndef NO_TCLTK
+.PHONY: .FORCE-GIT-GUI-VARS
+endif
 
 ### Check documentation
 #
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 733c07e..4ef6fac 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -40,10 +40,13 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
+git-gui: GIT-GUI-VARS
+
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	$(QUIET_GEN)rm -f $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 		-e 's/@@GITGUI_VERSION@@/$(GITGUI_VERSION)/g' \
+		-e'1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' \
 		$@.sh >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
@@ -66,7 +69,17 @@ dist-version:
 	@echo $(GITGUI_VERSION) > $(TARDIR)/version
 
 clean::
-	rm -f $(ALL_PROGRAMS) GIT-VERSION-FILE
+	rm -f $(ALL_PROGRAMS) GIT-VERSION-FILE GIT-GUI-VARS
+
+### Detect Tck/Tk interpreter path changes
+TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
+
+GIT-GUI-VARS: .FORCE-GIT-GUI-VARS
+	@VARS='$(TRACK_VARS)'; \
+	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+		echo 1>&2 "    * new Tcl/Tk interpreter location"; \
+		echo "$$VARS" >$@; \
+            fi
 
 .PHONY: all install dist-version clean
-.PHONY: .FORCE-GIT-VERSION-FILE
+.PHONY: .FORCE-GIT-VERSION-FILE .FORCE-GIT-GUI-VARS
-- 
1.5.0.3-dirty

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> --with-tcltk enables the search of the Tcl/Tk interpreter. If no
> interpreter is found then Tcl/Tk dependend parts are disabled.
>
> --without-tcltk unconditionally disables Tcl/Tk dependent parts.
>
> The original behaviour is not changed: bare './configure' just
> installs the Tcl/Tk part doing no checks for the interpreter.
>
> Makefile knob named NO_TCLTK was introduced. It prevents the build
> and installation of the Tcl/Tk dependent parts.
>
> Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
> ---

Thanks.

Is this supposed to be the first in the series?  I thought you
said you were going to do NO_TCLTK without anything else at all
first, and then TCLTK_PATH patch.  I am a bit lost here.

> +# Define NO_TCLTK if you do not want Tcl/Tk GUI.
> +#
> +# The TCLTK_PATH variable governs the location of the Tck/Tk interpreter.
> +# If not set it defaults to the bare 'wish'. If it is set to the empty
> +# string then NO_TCLTK will be forced (this is used by configure script).
> +#

Grumble.  If you are doing this, then there is not much point to
have two separate patches, is it?

> @@ -684,7 +696,9 @@ ifneq (,$X)
>  endif
>  
>  all::
> +ifndef NO_TCLTK
>  	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
> +endif
>  	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
>  	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)

Although you were not supposed to be talking about paths, since
you've already introduced TCLTK_PATH, it should be passed down
to git-gui here, I think.

> @@ -955,11 +974,17 @@ dist: git.spec git-archive
>  	@mkdir -p $(GIT_TARNAME)
>  	@cp git.spec $(GIT_TARNAME)
>  	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
> +ifndef NO_TCLTK
>  	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
>  	$(TAR) rf $(GIT_TARNAME).tar \
>  		$(GIT_TARNAME)/git.spec \
>  		$(GIT_TARNAME)/version \
>  		$(GIT_TARNAME)/git-gui/version
> +else
> +	$(TAR) rf $(GIT_TARNAME).tar \
> +		$(GIT_TARNAME)/git.spec \
> +		$(GIT_TARNAME)/version
> +endif
>  	@rm -rf $(GIT_TARNAME)
>  	gzip -f -9 $(GIT_TARNAME).tar
>  

Why should a source distribution exclude git-gui/ directory?  I
think it is sensible to ship a source that contains all.  You
are shipping gitk even without NO_TCLTK anyway, too.

And from the part 2:

> @@ -705,6 +709,12 @@ endif
>  strip: $(PROGRAMS) git$X
>  	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
>  
> +gitk-wish: gitk GIT-GUI-VARS
> +	$(QUIET_GEN)rm -f $@ $@+ && \
> +	sed -e'1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' < gitk > $@+ && \
> +	chmod +x $@+ && \
> +	mv -f $@+ $@
> +

This subst() is a nice attention to the detail.  I like it,
although in practice I do not think anybody is insane enough to
have a pipe character in the directory name that leads to wish.

I separated your two patches into three with minor modifications
and parked them in 'pu'.  We need to arrange with Shawn when to
apply the git-gui/ parts of the patch to his tree, but we are
not in a rush.

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Eygene Ryabinkin]

Junio, good day.

Wed, Mar 28, 2007 at 12:48:45PM -0700, Junio C Hamano wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
> 
> > --with-tcltk enables the search of the Tcl/Tk interpreter. If no
> > interpreter is found then Tcl/Tk dependend parts are disabled.
> >
> > --without-tcltk unconditionally disables Tcl/Tk dependent parts.
> >
> > The original behaviour is not changed: bare './configure' just
> > installs the Tcl/Tk part doing no checks for the interpreter.
> >
> > Makefile knob named NO_TCLTK was introduced. It prevents the build
> > and installation of the Tcl/Tk dependent parts.
> >
> > Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
> > ---
> 
> Thanks.
> 
> Is this supposed to be the first in the series?  I thought you
> said you were going to do NO_TCLTK without anything else at all
> first, and then TCLTK_PATH patch.  I am a bit lost here.
>
> > +# Define NO_TCLTK if you do not want Tcl/Tk GUI.
> > +#
> > +# The TCLTK_PATH variable governs the location of the Tck/Tk interpreter.
> > +# If not set it defaults to the bare 'wish'. If it is set to the empty
> > +# string then NO_TCLTK will be forced (this is used by configure script).
> > +#
> 
> Grumble.  If you are doing this, then there is not much point to
> have two separate patches, is it?

I cheated, sorry: first patch prepared the configure's infrastructure
for the --with-tcltk/--without-tcltk including --with-tcltk=PATH.

And the second one introduced the TCLTK_PATH usage for substituting
the 'wish' in the Tcl/Tk tools.

Sorry for the confusion.

> 
> > @@ -684,7 +696,9 @@ ifneq (,$X)
> >  endif
> >  
> >  all::
> > +ifndef NO_TCLTK
> >  	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
> > +endif
> >  	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
> >  	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
> 
> Although you were not supposed to be talking about paths, since
> you've already introduced TCLTK_PATH, it should be passed down
> to git-gui here, I think.

Yes, you're perfectly right.

> 
> > @@ -955,11 +974,17 @@ dist: git.spec git-archive
> >  	@mkdir -p $(GIT_TARNAME)
> >  	@cp git.spec $(GIT_TARNAME)
> >  	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
> > +ifndef NO_TCLTK
> >  	@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
> >  	$(TAR) rf $(GIT_TARNAME).tar \
> >  		$(GIT_TARNAME)/git.spec \
> >  		$(GIT_TARNAME)/version \
> >  		$(GIT_TARNAME)/git-gui/version
> > +else
> > +	$(TAR) rf $(GIT_TARNAME).tar \
> > +		$(GIT_TARNAME)/git.spec \
> > +		$(GIT_TARNAME)/version
> > +endif
> >  	@rm -rf $(GIT_TARNAME)
> >  	gzip -f -9 $(GIT_TARNAME).tar
> >  
> 
> Why should a source distribution exclude git-gui/ directory?  I
> think it is sensible to ship a source that contains all.  You
> are shipping gitk even without NO_TCLTK anyway, too.

Oops: didn't noticed that it is the tarball construction.

> And from the part 2:
> 
> > @@ -705,6 +709,12 @@ endif
> >  strip: $(PROGRAMS) git$X
> >  	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
> >  
> > +gitk-wish: gitk GIT-GUI-VARS
> > +	$(QUIET_GEN)rm -f $@ $@+ && \
> > +	sed -e'1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' < gitk > $@+ && \
> > +	chmod +x $@+ && \
> > +	mv -f $@+ $@
> > +
> 
> This subst() is a nice attention to the detail.  I like it,
> although in practice I do not think anybody is insane enough to
> have a pipe character in the directory name that leads to wish.

Thanks! And for the sanity: I do not think that the single quote
in the path it sane too. But as I was teached, "if we should
quote something, we must quote it". ;))

> 
> I separated your two patches into three with minor modifications
> and parked them in 'pu'.  We need to arrange with Shawn when to
> apply the git-gui/ parts of the patch to his tree, but we are
> not in a rush.

Thank you. Examined the 'origin/pu' and saw that you're already
incorporated the git.spec.in patch. I've found a glitch in it:
the right PYTHON_PATH should be passed. The patch follows.

By the way, when I was creating the git.spec from the git.spec.in,
I had the 'Version' field equal to the '1.5.1-rc1.GIT' and RPM
does not like the '-' characters inside the versions. Did
'tr - _' for specfile version and tarball name. The patch
follows.
-- 
Eygene

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Wed, Mar 28, 2007 at 12:48:45PM -0700, Junio C Hamano wrote:
>> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
>> ...
>> > +gitk-wish: gitk GIT-GUI-VARS
>> > +	$(QUIET_GEN)rm -f $@ $@+ && \
>> > +	sed -e'1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' < gitk > $@+ && \
>> > +	chmod +x $@+ && \
>> > +	mv -f $@+ $@
>> ...
> Thanks! And for the sanity: I do not think that the single quote
> in the path it sane too. But as I was teached, "if we should
> quote something, we must quote it". ;))

Actually, look at the wish script you are running sed on.

	exec wish "$0" -- "$@"

If you substitute "wish" with "/i use stupid/$PATH/to/wish", I
think Tcl splits the path at SP and does not protect $var
reference, so the careful quoting in the Makefile is still not
good enough ;-).

But come to think of it, it lets shell handle $PATH to find wish
anyway, so *unless* we have specific version dependency to wish
that wish binary normally found on user's $PATH is inadequate,
we probably should not even need to be doing any of this path
munging.  You might end up discovering the path to wish binary
in your autoconf script, we do not have to use it.  ./configure
can just see if there is wish, and set NO_TCLTK appropriately
without any of the path business.

What do you think?

> By the way, when I was creating the git.spec from the git.spec.in,
> I had the 'Version' field equal to the '1.5.1-rc1.GIT' and RPM
> does not like the '-' characters inside the versions.

That is semi-intended, in that you are not even supposed to be
building with "1.5.1-rc1.GIT".  The version file in the tarball
that git.spec file lives in should use git-describe, built from
the source before the tarball was made, to get the version
number, and wouldn't be "$anything.GIT", which is the last-ditch
fallback string, which is set by GIT-VERSION-GEN for people who
build in a wrong way.

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Eygene Ryabinkin]

Junio,
Thu, Mar 29, 2007 at 01:00:35AM -0700, Junio C Hamano wrote:
> Actually, look at the wish script you are running sed on.
> 
> 	exec wish "$0" -- "$@"
> 
> If you substitute "wish" with "/i use stupid/$PATH/to/wish", I
> think Tcl splits the path at SP and does not protect $var
> reference, so the careful quoting in the Makefile is still not
> good enough ;-).

It is not Tcl/Tk, who interprets that string: it is for shell.
So, if the line will look like
exec "/insane path/to/wish" "$0" -- "$@",
then we will just get the "/insane path/to/wish" executed with
the script name on the first place and other arguments following
the '--'.

Or you meant something different? I am little confused with
the '$PATH' in your example. Was it intended?

> But come to think of it, it lets shell handle $PATH to find wish
> anyway, so *unless* we have specific version dependency to wish
> that wish binary normally found on user's $PATH is inadequate,
> we probably should not even need to be doing any of this path
> munging.  You might end up discovering the path to wish binary
> in your autoconf script, we do not have to use it.  ./configure
> can just see if there is wish, and set NO_TCLTK appropriately
> without any of the path business.
> 
> What do you think?

There are problems at least with FreeBSD: it just installs the
wish8.4, wish8.3, wish8.2, etc. It does not provide the bare 'wish'
as the link to one of those: it is hard to tell what 'wish' we will
like to use. Sure, I can search for 'wish8.3', 'wish8.4' in the
configure script. But when new wish will be out the Git configure
should be fixed for it. Seems like passing the path of the Tcl/Tk
interpreter still have some meaning in this situation.

> 
> > By the way, when I was creating the git.spec from the git.spec.in,
> > I had the 'Version' field equal to the '1.5.1-rc1.GIT' and RPM
> > does not like the '-' characters inside the versions.
> 
> That is semi-intended, in that you are not even supposed to be
> building with "1.5.1-rc1.GIT".  The version file in the tarball
> that git.spec file lives in should use git-describe, built from
> the source before the tarball was made, to get the version
> number, and wouldn't be "$anything.GIT", which is the last-ditch
> fallback string, which is set by GIT-VERSION-GEN for people who
> build in a wrong way.

Just built the tarball and tried the produced specfile: it wanted
to build 'git-1.5.1.rc1.26.g7a88-dirty'. Yes, my repository was
dirty, I admit it. Maybe you're right and there is no good reason
for the '-' symbols in the version string.
-- 
Eygene

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Or you meant something different? I am little confused with
> the '$PATH' in your example. Was it intended?

Yes, the dollar-sign-in-pathname is pretty much a part of my
pathological example. 

>> But come to think of it, it lets shell handle $PATH to find wish
>> anyway, so *unless* we have specific version dependency to wish
>> that wish binary normally found on user's $PATH is inadequate,
>> we probably should not even need to be doing any of this path
>> munging.  You might end up discovering the path to wish binary
>> in your autoconf script, we do not have to use it.  ./configure
>> can just see if there is wish, and set NO_TCLTK appropriately
>> without any of the path business.
>> 
>> What do you think?
>
> There are problems at least with FreeBSD: it just installs the
> wish8.4, wish8.3, wish8.2, etc. It does not provide the bare 'wish'
> as the link to one of those.

Then sed -e 's/wish/$(WISH_NAME)/', still letting the shell to
handle the path part, could be a simpler option.  I dunno.

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Eygene Ryabinkin]

Thu, Mar 29, 2007 at 01:35:55AM -0700, Junio C Hamano wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> writes:
> 
> > Or you meant something different? I am little confused with
> > the '$PATH' in your example. Was it intended?
> 
> Yes, the dollar-sign-in-pathname is pretty much a part of my
> pathological example. 

Got it. So you mean that the '$' sign should be escaped as well? ;))
Or we can get another _SQ substitution to the script and the
string will look like
exec 'whatever you'\''d written here' "$0" -- "$@"

> >> But come to think of it, it lets shell handle $PATH to find wish
> >> anyway, so *unless* we have specific version dependency to wish
> >> that wish binary normally found on user's $PATH is inadequate,
> >> we probably should not even need to be doing any of this path
> >> munging.  You might end up discovering the path to wish binary
> >> in your autoconf script, we do not have to use it.  ./configure
> >> can just see if there is wish, and set NO_TCLTK appropriately
> >> without any of the path business.
> >> 
> >> What do you think?
> >
> > There are problems at least with FreeBSD: it just installs the
> > wish8.4, wish8.3, wish8.2, etc. It does not provide the bare 'wish'
> > as the link to one of those.
> 
> Then sed -e 's/wish/$(WISH_NAME)/', still letting the shell to
> handle the path part, could be a simpler option.  I dunno.

Ah, you mean that './configure --with-tcltk=wish8.4' should also
do the trick? It seems to be easy to achieve by just skipping
the 'test -x' part in the configure.ac. So the semantics of
'--with-tcltk=PATH' will be:
"If you're telling me about the path to the interpreter, it is
you who should take care of it. I do not mind if you will give
me something unexecutable, unexistent and so on.". Comments?
-- 
Eygene

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Tom Prince]

On Thu, Mar 29, 2007 at 12:58:35PM +0400, Eygene Ryabinkin wrote:
> Ah, you mean that './configure --with-tcltk=wish8.4' should also
> do the trick? It seems to be easy to achieve by just skipping
> the 'test -x' part in the configure.ac. So the semantics of
> '--with-tcltk=PATH' will be:
> "If you're telling me about the path to the interpreter, it is
> you who should take care of it. I do not mind if you will give
> me something unexecutable, unexistent and so on.". Comments?

Definitely, when cross compiling, or generating packages, you often
don't even have the program installed in the right place, so erroring
out is the wrong thing to do in this case.

  Tom

Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.

[Eygene Ryabinkin]

Tom, good day.

Thu, Mar 29, 2007 at 01:12:39PM +0400, Tom Prince wrote:
> Definitely, when cross compiling, or generating packages, you often
> don't even have the program installed in the right place, so erroring
> out is the wrong thing to do in this case.

OK, the patch follows.
-- 
Eygene

[PATCH] Eliminate checks of user-specified Tcl/Tk interpreter.

[Eygene Ryabinkin]

Do not make the checks on the Tcl/Tk interpreter passed by
'--with-tcltk=/path/to/wish' configure option: user is free to pass
anything.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 configure.ac |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 43a6769..50d2b85 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,8 +102,6 @@ if test -z "$NO_TCLTK"; then
   elif test "$with_tcltk" = "yes"; then
   # Tcl/Tk check requested.
     AC_CHECK_PROGS(TCLTK_PATH, [wish], )
-  elif ! test -x "$with_tcltk"; then
-    AC_MSG_ERROR([Tcl/Tk interpreter was not found in $with_tcltk])
   else
     AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])
     TCLTK_PATH="$with_tcltk"
-- 
1.5.0.3-dirty

[PATCH] Added correct Python path to the RPM specfile.

[Eygene Ryabinkin]

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 git.spec.in |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index e469f21..1e96eaa 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -1,4 +1,7 @@
 # Pass --without docs to rpmbuild if you don't want the documentation
+
+%define python_path /usr/bin/python
+
 Name: 		git
 Version: 	@@VERSION@@
 Release: 	1%{?dist}
@@ -93,12 +96,14 @@ Perl interface to Git
 
 %build
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" WITH_OWN_SUBPROCESS_PY=YesPlease \
-     WITH_P4IMPORT=YesPlease prefix=%{_prefix} all %{!?_without_docs: doc}
+     WITH_P4IMPORT=YesPlease PYTHON_PATH=%{python_path} prefix=%{_prefix} all \
+     %{!?_without_docs: doc}
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" DESTDIR=$RPM_BUILD_ROOT \
-     WITH_OWN_SUBPROCESS_PY=YesPlease WITH_P4IMPORT=YesPlease \
+     WITH_OWN_SUBPROCESS_PY=YesPlease \
+     WITH_P4IMPORT=YesPlease PYTHON_PATH=%{python_path} \
      prefix=%{_prefix} mandir=%{_mandir} INSTALLDIRS=vendor \
      install %{!?_without_docs: install-doc}
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
-- 
1.5.0.3-dirty

Re: [PATCH] Added make options NO_GUI and WITH_P4IMPORT.

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

>> Maybe later you might even want to view the graphical history
>> from the server displaying on remote X, who knows?
>
> Not sure: I don't like X on the servers ;)) But others can...

Did you know that you can have only minimum X clients on your
server machine, and display on remote X terminals?  But that is
besides the point.

>> We have NO_CURL and such because lack of the necessary libraries
>> and headers prevent your build from completing, but in the case
>> of git-gui and gitk, they are just scripts and you would not
>> have any trouble in building.  I do not know if adding more
>> conditional to Makefile in order to skip them is worth it.
>
> OK, I reworked the patch following the suggestion of Jakub and Johannes:
> now configure has the built-in detection of the Tcl/Tk binary and has
> the --with-tcltk/--without-tcltk options.

That is exactly what I am quite against.  I often find other
people's packages silly when they disable tk support only
because the build procedure does not find tcl/tk installed on
the system it is built on, even when the tk component of the
package is pure wish script and does not have any C native stuff
(which requires libtcl development component on the build
system, which in turn justifies such disabling).

> And regarding the building troubles and the additional knob in the
> Makefile: the trouble is in the packaging process. For example, RPM
> or FreeBSD ports are looking at what is really installed, so if
> user do not want the Tcl/Tk part, then no package parts that depend
> on it should be installed.

I think the simple RPM spec file we ship with git.git takes care
of that nicely by splitting gitk into a separate package (As it
was Chris Wright's work, I cannot take credit for that part at
all). I would imagine both modern distro's packaging system and
people who actually maintain packages for distros are capable
enough to handle this situation just fine.  I still do not think
the "packaging difficulty" is not a strong enough reason.  

But I could be persuaded otherwise...