Christian Stimming <stimming@xxxxxxx> writes: > -# From here on, these are needed in git.git/gitk/Makefile. > +prefix ?= $(HOME) > +bindir ?= $(prefix)/bin > +sharedir ?= $(prefix)/share > gitk_libdir ?= $(sharedir)/gitk/lib > msgsdir ?= $(gitk_libdir)/msgs > msgsdir_SQ = $(subst ','\'',$(msgsdir)) I see somwhat funny spacing there. I'd suggest giving up aligning with spaces and consistently saying "var ?= val" instead. I am reading the diff between the gitk-git/Makefile before and after merging gitk with your patch and the last three lines above are not context but additions from my point of view. > +install:: all > + $(INSTALL) gitk-wish '$(DESTDIR_SQ)$(bindir_SQ)'/gitk > + $(INSTALL) -d '$(DESTDIR_SQ)$(msgsdir_SQ)' > + $(foreach p,$(ALL_MSGFILES), $(INSTALL) $p '$(DESTDIR_SQ)$(msgsdir_SQ)' &&) true This is cute and correct (except I would have spelled "true" as ":" myself). I think we need to fix a few such constructs that use ";" instead of "&&" in Makefile in git.git. > +uninstall:: > + $(foreach p,$(ALL_MSGFILES), $(RM) '$(DESTDIR_SQ)$(msgsdir_SQ)'/$(notdir $p) &&) true > + $(RM) '$(DESTDIR_SQ)$(bindir_SQ)'/gitk I have a mild dislike against uninstall target, but that's Paul's call. > +clean:: > + $(RM) gitk-wish po/*.msg And this makes me wonder if the last token should be $(ALL_MSGFILES). Other than that, Ack from me. Thanks. - 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