Added make options NO_GUI and WITH_P4IMPORT.
PATCH 1/1 Added make options NO_GUI and WITH_P4IMPORT.
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.
--- Makefile | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
Makefile +44 -0
@@ -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' \ + [email protected] >$@+ + 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
Signature · 3 lines
1.5.0.3-dirty -- Eygene
Quoted 1 line · Eygene Ryabinkin
NO_GUIdisables 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.
Signature · 3 lines
Jakub Narebski Warsaw, Poland ShadeHawk on #git
Jakub, good day.
Quoted 7 lines · Jakub Narebski
Quoted 1 line · Eygene Ryabinkin
NO_GUIdisables 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?
Signature · 1 line
Eygene
Hi,
Quoted 3 lines · Eygene Ryabinkin
Will try to implement. But still,
NO_GUIshould 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
Johannes, good day.
Quoted 8 lines · Johannes Schindelin
Quoted 3 lines
Will try to implement. But still,
NO_GUIshould 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.
Quoted 3 lines
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.
Signature · 1 line
Eygene
Hi,
Quoted 5 lines · Eygene Ryabinkin
Quoted 4 lines · Johannes Schindelin
Quoted 3 lines
Will try to implement. But still,
NO_GUIshould 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?
Quoted 8 lines
Quoted 4 lines
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
Johannes,
Quoted 8 lines · Johannes Schindelin
Quoted 3 lines
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_GUIis a misnomer. It should beNO_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.
Quoted 4 lines
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,
_evenif_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.
Quoted 1 line
Conclusion: I am in favour of
NO_TCL, but not ofNO_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?
Signature · 1 line
Eygene
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
Johannes,
Quoted 9 lines · Johannes Schindelin
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'.
Quoted 7 lines
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?
Signature · 1 line
Eygene
Quoted 5 lines · Eygene Ryabinkin
Technically, the checks in
Makefilewill look as 'ifndefNO_GUI&&NO_TCL_TK' instead of 'ifndefNO_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.
Junio, good day.
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Eygene Ryabinkin
Technically, the checks in
Makefilewill look as 'ifndefNO_GUI&&NO_TCL_TK' instead of 'ifndefNO_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...
Quoted 5 lines
We have
NO_CURLand 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 toMakefilein 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 theMakefile: 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.
Signature · 1 line
Eygene
PATCH 1/1 Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.
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'.
--- Makefile | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++- config.mak.in | 1 + configure.ac | 19 ++++++++++++++++++ git-gui/Makefile | 1 + 4 files changed, 75 insertions(+), 2 deletions(-)
Makefile +54 -2
@@ -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' \ + [email protected] >$@+ + 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
config.mak.in +1 -0
@@ -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@
configure.ac +19 -0
@@ -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])
git-gui/Makefile +1 -0
@@ -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' ;)
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.
Thanks a lot for the patch, but...
Quoted 4 lines · Eygene Ryabinkin
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?
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.
Quoted 10 lines · Jakub Narebski
Thanks a lot for the patch, but...
Quoted 4 lines · Eygene Ryabinkin
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.
Jakub, Junio,
Quoted 4 lines · Junio C Hamano
Quoted 2 lines · Jakub Narebski
Thanks a lot for the patch, but...
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines · Eygene Ryabinkin
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.
Quoted 3 lines
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_TCLTKstuff 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.
Signature · 1 line
Eygene
Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.
Quoted 9 lines · Eygene Ryabinkin
Mon, Mar 26, 2007 at 01:36:49AM -0700, Junio C Hamano wrote:
...Quoted 3 lines
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_TCLTKstuff 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.
Junio, good day.
Quoted 12 lines · Junio C Hamano
Quoted 9 lines · Eygene Ryabinkin
Mon, Mar 26, 2007 at 01:36:49AM -0700, Junio C Hamano wrote:
...Quoted 3 lines
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_TCLTKstuff 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.
Quoted 7 lines
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?
Signature · 1 line
Eygene
PATCH 1/1 Added configure options --with-tcltk/--without-tcltk.
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.
--- Makefile | 30 ++++++++++++++++++++++++++++-- config.mak.in | 1 + configure.ac | 26 ++++++++++++++++++++++++++ git-gui/Makefile | 3 +++ 4 files changed, 58 insertions(+), 2 deletions(-)
Makefile +28 -2
@@ -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
config.mak.in +1 -0
@@ -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@
configure.ac +26 -0
@@ -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])
git-gui/Makefile +3 -0
@@ -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' ;)
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Added configure options --with-tcltk/--without-tcltk.
Quoted 7 lines · 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.
Quoted 5 lines
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 <[email protected]>
Thanks.
Quoted 10 lines
Makefile +2
-0@@ -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. +#
Makefile all::
Quoted 10 lines
Makefile +8
-0+# 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.
Makefile install: all · line 937
Quoted 10 lines
Makefile +5
-1@@ -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)'/gitkAlso 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.
Junio, good day.
Quoted 7 lines · Junio C Hamano
Makefile ifdef NO_PERL_MAKEMAKER · line 636
Quoted 4 lines
Makefile +4
-0+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.
Quoted 13 lines
Makefile install: all · line 937
Quoted 11 lines
Makefile +5
-1@@ -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)' +endifThis is a no-no. "make $args; su make $args install" should
Quoted 8 lines
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 theMakefile. For that purpose, I think the way $(SCRIPT_SH) are built using $(SHELL_PATH_SQ) can be learned from.
OK.
Quoted 3 lines
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?
Quoted 16 lines
You seem to have a grip on the use of
conditional inMakefileto 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
Quoted 3 lines
Also you need to rebuild gitk-wish when the builder gives different
TCLTK_PATH; I suspect the easiest way is tack it toTRACK_CFLAGSand make gitk-wish depend on GIT-CFLAGS.
OK, will try to provide the splitted patches.
Signature · 1 line
Eygene
Re: [PATCH] Added configure options --with-tcltk/--without-tcltk.
Quoted 7 lines · Eygene Ryabinkin
Quoted 3 lines
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 1/1 Add --with-tcltk and --without-tcltk to configure.
--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.
--- Makefile | 31 +++++++++++++++++++++++++++++-- config.mak.in | 1 + configure.ac | 26 ++++++++++++++++++++++++++ git-gui/Makefile | 3 +++ 4 files changed, 59 insertions(+), 2 deletions(-)
Makefile +29 -2
@@ -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
config.mak.in +1 -0
@@ -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@
configure.ac +26 -0
@@ -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])
git-gui/Makefile +3 -0
@@ -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 $@ $@+ && \
Signature · 1 line
1.5.0.3-dirty
PATCH 1/1 Added Tcl/Tk interpreter path rewriting for the GUI tools.
--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.
--- Makefile | 31 ++++++++++++++++++++++++++++--- git-gui/Makefile | 17 +++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-)
Makefile +28 -3
@@ -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 #
git-gui/Makefile +15 -2
@@ -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"|' \ [email protected] >$@+ && \ 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
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.
In reply to Eygene RyabinkinQuoted 5 lines · 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.
Quoted 8 lines
The original behaviour is not changed: bare './configure' just installs the Tcl/Tk part doing no checks for the interpreter.
Makefileknob namedNO_TCLTKwas introduced. It prevents the build and installation of the Tcl/Tk dependent parts.Signed-off-by Eygene Ryabinkin <[email protected]>---
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.
Makefile all:: · line 120
Quoted 6 lines
Makefile +6
-0+# 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?
Makefile ifneq (,$X) · line 703
Quoted 9 lines
Makefile +2
-0@@ -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.
Quoted 8 lines
Makefile +1
-0@@ -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 \
Makefile dist: git.spec git-archive · line 990
Quoted 10 lines
Makefile +5
-0$(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:
Review endif · line 717
Quoted 10 lines
(quoted diff) +6
-0@@ -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.
Junio, good day.
Quoted 22 lines · Junio C Hamano
Quoted 5 lines · 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.
Quoted 8 lines
The original behaviour is not changed: bare './configure' just installs the Tcl/Tk part doing no checks for the interpreter.
Makefileknob namedNO_TCLTKwas introduced. It prevents the build and installation of the Tcl/Tk dependent parts.Signed-off-by Eygene Ryabinkin <[email protected]>---
Thanks.
Is this supposed to be the first in the series? I thought you said you were going to do
NO_TCLTKwithout anything else at all first, and thenTCLTK_PATHpatch. I am a bit lost here.
Quoted 9 lines
Makefile all:: · line 120
Quoted 6 lines
Makefile +6
-0+# 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.
Quoted 4 lines
Quoted 4 lines
(quoted diff) +0
-0@@ -684,7 +696,9 @@ ifneq (,$X) endif
Quoted 10 lines
Makefile ifneq (,$X) · line 703
Quoted 6 lines
Makefile +2
-0all:: +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.
Quoted 15 lines
Quoted 15 lines
Makefile +5
-0@@ -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
Quoted 8 lines
Makefile dist: git.spec git-archive · line 990
Quoted 4 lines
Makefile +1
-0+endif @rm -rf $(GIT_TARNAME) gzip -f -9 $(GIT_TARNAME).tarWhy 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_TCLTKanyway, too.
Oops: didn't noticed that it is the tarball construction.
Quoted 6 lines
And from the part 2:
Quoted 4 lines
(quoted diff) +0
-0@@ -705,6 +709,12 @@ endif strip: $(PROGRAMS) git$X $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
Quoted 10 lines
Review endif · line 717
Quoted 6 lines
(quoted diff) +6
-0+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". ;))
Quoted 5 lines
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.
Signature · 1 line
Eygene
PATCH 1/1 Added correct Python path to the RPM specfile.
--- git.spec.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
git.spec.in +7 -2
@@ -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 {} ';'
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.
In reply to Eygene RyabinkinQuoted 9 lines · Eygene Ryabinkin
Quoted 8 lines · Junio C Hamano
Eygene Ryabinkin <[email protected]> writes:
...Quoted 5 lines
(quoted diff) +5
-0+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 $@+ $@...
Quoted 3 lines
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?
Quoted 3 lines
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.
Junio,
Quoted 8 lines · Junio C Hamano
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
Makefileis 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?
Quoted 10 lines
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_TCLTKappropriately 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.
Quoted 5 lines
Quoted 4 lines
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.
Quoted 7 lines
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.
Signature · 1 line
Eygene
Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.
Quoted 2 lines · Eygene Ryabinkin
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.
Quoted 9 lines
Quoted 9 lines
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_TCLTKappropriately without any of the path business.
Quoted 5 lines
Quoted 1 line
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.
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · Eygene Ryabinkin
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" -- "$@"
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
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_TCLTKappropriately without any of the path business.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
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?
Signature · 1 line
Eygene
Re: [PATCH] Add --with-tcltk and --without-tcltk to configure.
Quoted 7 lines · Eygene Ryabinkin
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.
Tom, good day.
Quoted 3 lines · Tom Prince
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.
Signature · 1 line
Eygene
PATCH 1/1 Eliminate checks of user-specified Tcl/Tk interpreter.
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.
--- configure.ac | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
configure.ac +0 -2
@@ -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"
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Added options NO_TCLTK, WITH_P4IMPORT and --with-tcltk/--without-tcltk.
In reply to Eygene RyabinkinPlease 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 1/1 Add the WITH_P4IMPORT knob to the Makefile.
WITH_P4IMPORT: enables the installation of the Perforce import script.
--- Makefile | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)
Makefile +26 -0
@@ -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' \ + [email protected] >$@+ + chmod +x $@+ + mv $@+ $@ + perl/perl.mak: GIT-CFLAGS $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
Signature · 1 line
1.5.0.3-dirty
Re: [PATCH] Add the WITH_P4IMPORT knob to the Makefile.
Quoted 4 lines · Eygene Ryabinkin
WITH_P4IMPORT: enables the installation of the Perforce import script.Signed-off-by Eygene Ryabinkin <[email protected]>
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.
Quoted 8 lines · Junio C Hamano
Quoted 4 lines · Eygene Ryabinkin
WITH_P4IMPORT: enables the installation of the Perforce import script.Signed-off-by Eygene Ryabinkin <[email protected]>Thanks.
You're welcome ;))
Quoted 2 lines
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.
Signature · 1 line
Eygene
PATCH 1/1 Added git-p4 package to the list of git RPMs.
--- git.spec.in | 25 +++++++++++++++++++++---- 1 files changed, 21 insertions(+), 4 deletions(-)
git.spec.in +21 -4
@@ -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 <[email protected]> +- Added the git-p4 package: Perforce import stuff. + * Mon Feb 13 2007 Nicolas Pitre <[email protected]> - Update core package description (Git isn't as stupid as it used to be)
Signature · 1 line
1.5.0.3-dirty
PATCH 1/1 Remove unused WITH_OWN_SUBPROCESS_PY from RPM spec
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(-)
git.spec.in +4 -5
@@ -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 {} ';'
Signature · 1 line
1.5.1.rc2
Re: [PATCH] Added git-p4 package to the list of git RPMs.
In reply to Eygene RyabinkinQuoted 6 lines · Eygene Ryabinkin
git.spec.in +0
-0@@ -50,6 +50,13 @@ Requires: git-core = %{version}-%{release}, tla %description arch
git.spec.in Requires: git-core = %{version}-%{release}, tla · line 59
Quoted 10 lines
git.spec.in +7
-0Git 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.
git.spec.in +1 -1
@@ -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.
Junio, good day.
Quoted 1 line · Junio C Hamano
Thanks. I'll also add git-p4 here.
Thank you.
Signature · 1 line
Eygene
Quoted 4 lines · Eygene Ryabinkin
Quoted 2 lines
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.
Quoted 9 lines
Quoted 5 lines
We have
NO_CURLand 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 toMakefilein 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).
Quoted 5 lines
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...
Quoted 9 lines
Quoted 4 lines
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,
_evenif_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
Paolo, good day.
Quoted 4 lines · Paolo Bonzini
Quoted 4 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 4 lines
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,
_evenif_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?
Signature · 1 line
Eygene
Quoted 7 lines
Quoted 4 lines
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_GUIorNO_TCLnames?
I agree with you. ;-)
Paolo
Quoted 10 lines · Eygene Ryabinkin
Quoted 9 lines · Paolo Bonzini
Quoted 8 lines
Quoted 3 lines
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,
_evenif_the dependency is there already.
Quoted 6 lines
Quoted 3 lines
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_GUIorNO_TCLnames?
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).
Quoted 8 lines · Alex Riesen
Quoted 2 lines · Eygene Ryabinkin
Yep, you're right. That was I meant originally. How do you feel about
NO_GUIorNO_TCLnames?That'd be
NO_TCL_TK. TCL has nothing to do with graphics. And you have one more supporter forNO_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).
Signature · 2 lines
Jakub Narebski Poland