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 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html