From: Norbert Nemec Date: Thu, 18 Nov 2010 07:35:29 GMT Subject: Re: patch for AIX system Message-ID: <4CE4D741.2070307@Nemec-online.de> In-Reply-To: <7vr5ejg7oi.fsf@alter.siamese.dyndns.org> Sorry about that and thanks for the patient reply. I'll crank up my standard of quality and address the issues with another submission. Greetings, Norbert On 11/17/2010 06:51 PM, Junio C Hamano wrote: > Norbert Nemec writes: > >> Find attached a small patch to make git install cleanly on an AIX system > A few comments, some from Documentation/SubmittingPatches. > >> From def4428ad827d1e6550634a2fe1f035c1b148426 Mon Sep 17 00:00:00 2001 >> From: Norbert Nemec >> Date: Wed, 17 Nov 2010 08:25:44 +0100 >> Subject: [PATCH] Fix for installation on AIX > Please don't do this; instead, make sure that From/Date/Subject of your > e-mail usable as the metainfo for the resulting commit (in this case only > the Subject needs a change) and drop these lines, i.e. make the patch > inline, not an attachment. > > Also make the subject more specific, to make it clear _what_ you fixed; > phrase it like "Makefile: On AIX, bsdinstall does not understand -d" or > something, perhaps. > >> The BSD style 'install' command is call 'installbsd' and does not support the -d option. >> Therefore '$(INSTALL) -d' is replaced by a new variable '$(INSTALLDIR)' in all Makefiles >> which can be changed independently of $(INSTALL). > These lines are tad too long; please wrap at around 66-72 cols. > > Missing sign-off. > >> --- >> Documentation/Makefile | 10 +++++----- >> Makefile | 7 +++++-- >> git-gui/Makefile | 4 ++-- >> gitk-git/Makefile | 2 +- >> gitweb/Makefile | 4 ++-- >> templates/Makefile | 2 +- >> 6 files changed, 16 insertions(+), 13 deletions(-) >> >> diff --git a/Documentation/Makefile b/Documentation/Makefile >> index e117bc4..c2db8db 100644 >> --- a/Documentation/Makefile >> +++ b/Documentation/Makefile >> ... >> install-pdf: pdf >> - $(INSTALL) -d -m 755 $(DESTDIR)$(pdfdir) >> + $(INSTALLDIR) -m 755 $(DESTDIR)$(pdfdir) >> $(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir) >> >> install-html: html > In the main Makefile, we have this line: > > export DIFF TAR INSTALL DESTDIR SHELL_PATH > > so that they can be used in "$(MAKE) -C Documentaiton install-frotz" > invocations in subdirectories, but I do not see an addition of export in > your patch, and you do not define INSTALLDIR in Documentation/Makefile > either. I wonder how this could possibly work... > >> diff --git a/Makefile b/Makefile >> index 1f1ce04..2664d7c 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -953,6 +954,8 @@ ifeq ($(uname_S),AIX) >> NO_MKSTEMPS = YesPlease >> NO_STRLCPY = YesPlease >> NO_NSEC = YesPlease >> + INSTALL = installbsd >> + INSTALLDIR = mkdir -p >> FREAD_READS_DIRECTORIES = UnfortunatelyYes >> INTERNAL_QSORT = UnfortunatelyYes >> NEEDS_LIBICONV=YesPlease > I'd defer this section to AIX experts; I've always assumed that people > on non-gnu platforms used ginstall, but perhaps AIX doesn't have one? > > Other changes looked mostly OK, but I am still wondering how your > INSTALLDIR is passed down to submakes. > > ... spends quality 30-minutes digging ... > > No, your patch does not work. Have you even tested it? > > The attached patch on top of yours makes things working, it seems. > > Points to note: > > - People may have already used "make INSTALL=ginstall" and been expecting > it to work. Defining "INSTALLDIR = install -d" as the default will > break them (see how I defined it in the attached); > > - "cd gitweb&& make" is supposed to work, so you would need fallback > definitions in Makefiles in subdirectories; and > > - When coming from the toplevel make, submakes do want the INSTALLDIR > definition passed from it (see how "export" is used). > > Thanks. > > Documentation/Makefile | 3 ++- > Makefile | 4 ++-- > git-gui/Makefile | 3 +++ > gitk-git/Makefile | 1 + > gitweb/Makefile | 1 + > templates/Makefile | 1 + > 6 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/Documentation/Makefile b/Documentation/Makefile > index c2db8db..5c8aacc 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -44,7 +44,8 @@ ASCIIDOC=asciidoc > ASCIIDOC_EXTRA = > MANPAGE_XSL = manpage-normal.xsl > XMLTO_EXTRA = > -INSTALL?=install > +INSTALL ?= install > +INSTALLDIR ?= $(INSTALL) -d > RM ?= rm -f > DOC_REF = origin/man > HTML_REF = origin/html > diff --git a/Makefile b/Makefile > index 2664d7c..b91de40 100644 > --- a/Makefile > +++ b/Makefile > @@ -308,7 +308,7 @@ DIFF = diff > TAR = tar > FIND = find > INSTALL = install > -INSTALLDIR = install -d > +INSTALLDIR = $(INSTALL) -d > RPMBUILD = rpmbuild > TCL_PATH = tclsh > TCLTK_PATH = wish > @@ -1573,7 +1573,7 @@ endif > ALL_CFLAGS += $(BASIC_CFLAGS) > ALL_LDFLAGS += $(BASIC_LDFLAGS) > > -export DIFF TAR INSTALL DESTDIR SHELL_PATH > +export DIFF TAR INSTALL INSTALLDIR DESTDIR SHELL_PATH > > > ### Build rules > diff --git a/git-gui/Makefile b/git-gui/Makefile > index b96b3df..be8bd2d 100644 > --- a/git-gui/Makefile > +++ b/git-gui/Makefile > @@ -44,6 +44,9 @@ endif > ifndef INSTALL > INSTALL = install > endif > +ifndef INSTALLDIR > + INSTALLDIR = $(INSTALL) -d > +endif > > RM_RF ?= rm -rf > RMDIR ?= rmdir > diff --git a/gitk-git/Makefile b/gitk-git/Makefile > index b838d87..473c500 100644 > --- a/gitk-git/Makefile > +++ b/gitk-git/Makefile > @@ -11,6 +11,7 @@ msgsdir_SQ = $(subst ','\'',$(msgsdir)) > TCL_PATH ?= tclsh > TCLTK_PATH ?= wish > INSTALL ?= install > +INSTALLDIR ?= $(INSTALL) -d > RM ?= rm -f > > DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) > diff --git a/gitweb/Makefile b/gitweb/Makefile > index 3f1ac82..ee6dcc1 100644 > --- a/gitweb/Makefile > +++ b/gitweb/Makefile > @@ -16,6 +16,7 @@ gitwebdir ?= /var/www/cgi-bin > > RM ?= rm -f > INSTALL ?= install > +INSTALLDIR ?= $(INSTALL) -d > > # default configuration for gitweb > GITWEB_CONFIG = gitweb_config.perl > diff --git a/templates/Makefile b/templates/Makefile > index 3808b04..953037c 100644 > --- a/templates/Makefile > +++ b/templates/Makefile > @@ -5,6 +5,7 @@ ifndef V > endif > > INSTALL ?= install > +INSTALLDIR ?= $(INSTALL) -d > TAR ?= tar > RM ?= rm -f > prefix ?= $(HOME) -- _______________________________________________Norbert Nemec Lilienstr. 5a ... 12203 Berlin-Lichterfelde ... Germany Tel: +49-30-5483 3143 Mobile: +49-176-5502 5643 eMail: