Norbert Nemec <Norbert@xxxxxxxxxxxxxxx> 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 <Norbert@xxxxxxxxxxxxxxx> > 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) -- 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