threads / patch / 11539

patchgitk: Update and fix Makefile

Subject: [PATCH] gitk: Update and fix Makefile

## tl;dr

10 messages between Jan 8, 2008 and Jan 13, 2008. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Christian Stimming· Jan 8, 2008, 20:54 UTC · lore

This Makefile uses the template provided at git.git/gitk-git/Makefile by Junio and adds the rules for the i18n files.

Signed-off-by: Christian Stimming <stimming@tuhh.de>
---
This patch is against master of git.kernel.org.
@Paul: Ignore my previous patch - "make install" didn't 
work there, but it does here.
 Makefile |   63 +++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 38 insertions(+), 25 deletions(-)
Show changes to Makefile +38 −25
diff --git a/Makefile b/Makefile
index 38154b6..6158504 100644
--- a/Makefile
+++ b/Makefile
@@ -1,25 +1,52 @@
-# Set the installation directories; this section is needed only in
-# gitk.git but probably not in git.git.
-ifndef gitexecdir
-	gitexecdir := $(shell git --exec-path)
-endif
-ifndef sharedir
-	sharedir := $(dir $(gitexecdir))share
-endif
+# The default target of this Makefile is...
+all::
 
-# 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))
 
-## Beginning of po-file creation rules
+TCLTK_PATH ?= wish
+INSTALL ?= install
+RM ?= rm -f
+
+DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
+bindir_SQ = $(subst ','\'',$(bindir))
+TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
+
+## po-file creation rules
 XGETTEXT   ?= xgettext
 MSGFMT     ?= msgfmt
 PO_TEMPLATE = po/gitk.pot
 ALL_POFILES = $(wildcard po/*.po)
 ALL_MSGFILES = $(subst .po,.msg,$(ALL_POFILES))
 
-all:: $(ALL_MSGFILES)
+ifndef V
+	QUIET          = @
+	QUIET_GEN      = $(QUIET)echo '   ' GEN $@ &&
+endif
+
+all:: gitk-wish $(ALL_MSGFILES)
+
+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
+
+uninstall::
+	$(foreach p,$(ALL_MSGFILES), $(RM) '$(DESTDIR_SQ)$(msgsdir_SQ)'/$(notdir $p) &&) true
+	$(RM) '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
+
+clean::
+	$(RM) gitk-wish po/*.msg
+
+gitk-wish: gitk
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' <gitk >$@+ && \
+	chmod +x $@+ && \
+	mv -f $@+ $@
 
 $(PO_TEMPLATE): gitk
 	$(XGETTEXT) -kmc -LTcl -o $@ gitk
@@ -29,17 +56,3 @@ $(ALL_MSGFILES): %.msg : %.po
 	@echo Generating catalog $@
 	$(MSGFMT) --statistics --tcl $< -l $(basename $(notdir $<)) -d $(dir $@)
 
-clean::
-	rm -f $(ALL_PROGRAMS) po/*.msg
-## End of po-file creation rules
-
-# Install rules for po-files
-install: all
-	$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(msgsdir_SQ)' $(INSTALL_D1)
-	$(QUIET)$(foreach p,$(ALL_MSGFILES), $(INSTALL_R0)$p $(INSTALL_R1) '$(DESTDIR_SQ)$(msgsdir_SQ)' &&) true
-
-uninstall:
-	$(QUIET)$(foreach p,$(ALL_MSGFILES), $(REMOVE_F0)'$(DESTDIR_SQ)$(msgsdir_SQ)'/$(notdir $p) $(REMOVE_F1) &&) true
-	$(QUIET)$(REMOVE_D0)'$(DESTDIR_SQ)$(msgsdir_SQ)' $(REMOVE_D1)
-	$(QUIET)$(REMOVE_D0)'$(DESTDIR_SQ)$(libdir_SQ)' $(REMOVE_D1)
-	$(QUIET)$(REMOVE_D0)`dirname '$(DESTDIR_SQ)$(libdir_SQ)'` $(REMOVE_D1)

-- 
1.5.3.4.206.g58ba4
Junio C Hamano· Jan 8, 2008, 22:54 UTC · re: Christian Stimming · lore

Re: [PATCH] gitk: Update and fix Makefile

Christian Stimming <stimming@tuhh.de> writes:
Show 7 quoted lines
> -# 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.
Paul Mackerras· Jan 9, 2008, 03:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: Update and fix Makefile

Junio C Hamano writes:
> I see somwhat funny spacing there.  I'd suggest giving up
> aligning with spaces and consistently saying "var ?= val"
> instead.

I made those lines all have one space before and after the ?=, and committed Christian's patches (plus one from Gerrit Pape), and pushed it out. Please do a pull.

Paul.
Junio C Hamano· Jan 9, 2008, 08:19 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: Update and fix Makefile

Paul Mackerras <paulus@samba.org> writes:
Show 9 quoted lines
> Junio C Hamano writes:
>
>> I see somwhat funny spacing there.  I'd suggest giving up
>> aligning with spaces and consistently saying "var ?= val"
>> instead.
>
> I made those lines all have one space before and after the ?=, and
> committed Christian's patches (plus one from Gerrit Pape), and pushed
> it out.  Please do a pull.
There were a couple of things I noticed that made me somewhat curious:
 * There are more spaces around ?= in Christian's patch to Makefile.
 * You have two patches from Christian and one patch from
   Gerrit; the author and commit timestamps of these commits are
   the same and in your timezone.
but nothing to complain about.  Pulled and pushed out.
Junio C Hamano· Jan 11, 2008, 08:57 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: Update and fix Makefile

I applied three patches since last pull from you and pushed the result (pure gitk part) out in gitk branch:

	master.kernel.org:/pub/scm/git/git.git/ gitk
Could you please pull?
Junio C Hamano· Jan 11, 2008, 09:06 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: Update and fix Makefile

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> I applied three patches since last pull from you and pushed the
> result (pure gitk part) out in gitk branch:
>
> 	master.kernel.org:/pub/scm/git/git.git/ gitk
>
> Could you please pull?

Sorry, this was a very ill-behaved pull request. The patches picked up from the list I applied are these three:

Charles Bailey (1):
      gitk: Fix the Makefile to cope with systems lacking msgfmt
Christian Stimming (2):
      gitk: Fix typo in user message.
      gitk: Update German translation.
Paul Mackerras· Jan 11, 2008, 23:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: Update and fix Makefile

Junio C Hamano writes:
Show 8 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I applied three patches since last pull from you and pushed the
> > result (pure gitk part) out in gitk branch:
> >
> > 	master.kernel.org:/pub/scm/git/git.git/ gitk
> >
> > Could you please pull?
I get:

~/gitk$ git pull master.kernel.org:/pub/scm/git/git.git/ gitk error: no such remote ref refs/heads/gitk fatal: Fetch failure: master.kernel.org:/pub/scm/git/git.git/

Show 8 quoted lines
> Sorry, this was a very ill-behaved pull request.  The patches
> picked up from the list I applied are these three:
> 
> Charles Bailey (1):
>       gitk: Fix the Makefile to cope with systems lacking msgfmt
> 
> Christian Stimming (2):
>       gitk: Fix typo in user message.

I was going to ignore this one since "descendent" is actually a valid alternate spelling, and is the one I am used to. However, I don't have a strong feeling about it.

Paul.
Junio C Hamano· Jan 11, 2008, 23:57 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: Update and fix Makefile

Paul Mackerras <paulus@samba.org> writes:
Show 5 quoted lines
> I get:
>
> ~/gitk$ git pull master.kernel.org:/pub/scm/git/git.git/ gitk
> error: no such remote ref refs/heads/gitk
> fatal: Fetch failure: master.kernel.org:/pub/scm/git/git.git/

My bash history tells me that I only did push --dry-run. Stupid me. Sorry about the noise.

Show 6 quoted lines
>> Christian Stimming (2):
>>       gitk: Fix typo in user message.
>
> I was going to ignore this one since "descendent" is actually a valid
> alternate spelling, and is the one I am used to.  However, I don't
> have a strong feeling about it.

I agree with you that both are valid spellings, but when I received 9e5d87d49070fe0463040e826824d6ce41beb089, I consulted a couple of dictionaries and descendant seemed to be more widely used. Besides, I think the german translation update depends on it ;-).

Anyway, I pushed it (this time without --dry-run) to
    master.kernel.org:/pub/scm/git/git.git/ gitk-for-paulus
Sorry, and thanks.
Jeff King· Jan 12, 2008, 05:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: Update and fix Makefile

On Fri, Jan 11, 2008 at 03:57:34PM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> I agree with you that both are valid spellings, but when I
> received 9e5d87d49070fe0463040e826824d6ce41beb089, I consulted a
> couple of dictionaries and descendant seemed to be more widely
> used.  Besides, I think the german translation update depends on
> it ;-).

Descendant is a noun, whereas descendent is an adjective. Though like many such distinctions, the two forms increasingly are used interchangeably.

-Peff
Christian Stimming· Jan 13, 2008, 21:06 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: Update and fix Makefile (spelling fix)

Am Samstag, 12. Januar 2008 00:36 schrieb Paul Mackerras:
Show 6 quoted lines
> > Christian Stimming (2):
> >       gitk: Fix typo in user message.
>
> I was going to ignore this one since "descendent" is actually a valid
> alternate spelling, and is the one I am used to.  However, I don't
> have a strong feeling about it.

In the dictionaries I consulted before proposing this patch, "descendant" was the *only* valid spelling. I checked http://dict.leo.org/?search=descendent , (on paper) Oxford Advanced Learner's Dictionary (1998 edition). Now when looking up http://www.m-w.com/dictionary/descendent your original spelling is also mentioned as a variant, but as the other dictionaries didn't have it, I assumed this should rather be changed.

Christian

← back to recent threads