Re: [PATCH] wincred: add install target and avoid overwriting configured variables

From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Apr 30, 2014, 09:21 UTC
Message-ID: <CABPQNSZsviaGqFeKZE4ofF6HoUQrPvNPuowar4YDjk_Mbu5iCQ@mail.gmail.com>
In-Reply-To: <20140430064610.GA22094@camelia.ucw.cz>

On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal <kasal@ucw.cz> wrote:

Show 9 quoted lines

> From: Pat Thoyts <patthoyts@users.sourceforge.net>
> Date: Wed, 24 Oct 2012 00:15:29 +0100
>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
> ---
> Another one from msysGit project.
> Original subject by Pat; I would suggest:
>     wincred: improve Makefile

I'm a little bit unsure about this, because the makefile was basically just copied from contrib/credential/osxkeychain/Makefile (which was the first credential helper) and tweaked slightly.

So, what makes wincred special compared to gnome-keyring, netrc and osxkeychain wrt installation? Shouldn't all helpers get the same treatment?

Show 24 quoted lines

>  contrib/credential/wincred/Makefile | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
> index bad45ca..3ce6aba 100644
> --- a/contrib/credential/wincred/Makefile
> +++ b/contrib/credential/wincred/Makefile
> @@ -1,14 +1,20 @@
> -all: git-credential-wincred.exe
> -
> -CC = gcc
> -RM = rm -f
> -CFLAGS = -O2 -Wall
> -
>  -include ../../../config.mak.autogen
>  -include ../../../config.mak
>
> -git-credential-wincred.exe : git-credential-wincred.c
> +prefix ?= /usr/local
> +libexecdir ?= $(prefix)/libexec/git-core
> +
> +INSTALL ?= install
> +
> +GIT_CREDENTIAL_WINCRED := git-credential-wincred.exe

Why this variable? IMO, it's just as "GIT_CREDENTIAL_WINCRED" easy to miss-spell as "git-credential-wincred.exe", and it doesn't seem to be possible to overload.

> +
> +all: $(GIT_CREDENTIAL_WINCRED)
> +

Also, why move the all-target down from the top? Is it simply because of the definition above?