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

> > +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.

> 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.