git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] wincred: add install target and avoid overwriting configured variables
@ 2014-04-30  6:46 Stepan Kasal
  2014-04-30  9:21 ` Erik Faye-Lund
  0 siblings, 1 reply; 12+ messages in thread
From: Stepan Kasal @ 2014-04-30  6:46 UTC (permalink / raw)
  To: GIT Mailing-list

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
 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
+
+all: $(GIT_CREDENTIAL_WINCRED)
+
+$(GIT_CREDENTIAL_WINCRED): git-credential-wincred.c
 	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
+install: $(GIT_CREDENTIAL_WINCRED)
+	$(INSTALL) -m 755 $(GIT_CREDENTIAL_WINCRED) $(libexecdir)
+
 clean:
-	$(RM) git-credential-wincred.exe
+	$(RM) $(GIT_CREDENTIAL_WINCRED)
-- 
1.9.2.msysgit.0.158.g6070cee

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] wincred: add install target and avoid overwriting configured variables
  2014-04-30  6:46 [PATCH] wincred: add install target and avoid overwriting configured variables Stepan Kasal
@ 2014-04-30  9:21 ` Erik Faye-Lund
  2014-04-30 11:27   ` Stepan Kasal
  0 siblings, 1 reply; 12+ messages in thread
From: Erik Faye-Lund @ 2014-04-30  9:21 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list

On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal <kasal@ucw.cz> wrote:
> 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?

>  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?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wincred: add install target and avoid overwriting configured variables
  2014-04-30  9:21 ` Erik Faye-Lund
@ 2014-04-30 11:27   ` Stepan Kasal
  2014-04-30 11:56     ` Erik Faye-Lund
  0 siblings, 1 reply; 12+ messages in thread
From: Stepan Kasal @ 2014-04-30 11:27 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: GIT Mailing-list, Pat Thoyts

Hello,

> On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal <kasal@ucw.cz> wrote:
> > 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

On Wed, Apr 30, 2014 at 11:21:17AM +0200, Erik Faye-Lund wrote:
> 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?

I can only guess that the hardwired CC and CFLAGS values can cause
problems.
It is probably much sane on Windows to use the compiler that the user
set up for the build.

I'm not sure if users of, say, OS X, have the same problems, so I
would hesitate to apply these changes to all helpers.

> > From: Pat Thoyts <patthoyts@users.sourceforge.net>
> >  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.

If you mis-spell a variable name, nothing is build.  If you misspell
a binary name, that binary may get compiled using a default rule;
that is why I would generally prefer variables.
Moreover, if the cardinality of the set ever increases, the
indirection may get helpful.
No big deal.

> > +
> > +all: $(GIT_CREDENTIAL_WINCRED)
> > +
> 
> Also, why move the all-target down from the top? Is it simply because
> of the definition above?

Again, I agree with Pat that it is nicer this way, but no big
deal.

Stepan Kasal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wincred: add install target and avoid overwriting configured variables
  2014-04-30 11:27   ` Stepan Kasal
@ 2014-04-30 11:56     ` Erik Faye-Lund
  2014-05-13  5:59       ` [PATCH 0/2] wincred makefile changes Stepan Kasal
  0 siblings, 1 reply; 12+ messages in thread
From: Erik Faye-Lund @ 2014-04-30 11:56 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, Pat Thoyts

On Wed, Apr 30, 2014 at 1:27 PM, Stepan Kasal <kasal@ucw.cz> wrote:
> Hello,
>
>> On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal <kasal@ucw.cz> wrote:
>> > 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
>
> On Wed, Apr 30, 2014 at 11:21:17AM +0200, Erik Faye-Lund wrote:
>> 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?
>
> I can only guess that the hardwired CC and CFLAGS values can cause
> problems.

I doubt that a patch that doesn't describe exactly what kind of issues
will get merged. And it certainly won't get my ack unless I understand
why.

> It is probably much sane on Windows to use the compiler that the user
> set up for the build.

But you can already do that, the same way as for the rest of git, by
overloading these in config.mak in the root of the git repo.

> I'm not sure if users of, say, OS X, have the same problems, so I
> would hesitate to apply these changes to all helpers.

Even if I bought that it's needed (which I'm currently skeptical to),
I think the "dunno about OSX" is a bit of a cop-out.

>> > From: Pat Thoyts <patthoyts@users.sourceforge.net>
>> >  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.
>
> If you mis-spell a variable name, nothing is build.  If you misspell
> a binary name, that binary may get compiled using a default rule;
> that is why I would generally prefer variables.

Following that logic, you should be submitting similar patches to the
main git Makefile as well. Somehow I doubt that'll happen.

> Moreover, if the cardinality of the set ever increases, the
> indirection may get helpful.

I don't think there's any reason to expect the number of binaries to
increase, so that's moot. And if I'm wrong, let's deal with that when
the time comes. It's not like this version is prepared for the
variable being a list either - neither should there IMO.

>> > +
>> > +all: $(GIT_CREDENTIAL_WINCRED)
>> > +
>>
>> Also, why move the all-target down from the top? Is it simply because
>> of the definition above?
>
> Again, I agree with Pat that it is nicer this way, but no big
> deal.

We don't usually do "this is subjectively better"-patches in Git.
Instead we try to follow the current style.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/2] wincred makefile changes
  2014-04-30 11:56     ` Erik Faye-Lund
@ 2014-05-13  5:59       ` Stepan Kasal
  2014-05-13  6:01         ` [PATCH 1/2] wincred: add install target Stepan Kasal
  2014-05-13  6:01         ` [PATCH 2/2] wincred: avoid overwriting configured variables Stepan Kasal
  0 siblings, 2 replies; 12+ messages in thread
From: Stepan Kasal @ 2014-05-13  5:59 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: GIT Mailing-list, Pat Thoyts, msysgit

Hello again,
  I revisited a patch that is present in msysgit since Oct 2012.
Dropping the cosmetic changes, there are two separate parts:

Pat Thoyts (2):
  wincred: add install target
  wincred: avoid overwriting configured variables

wincred: add install target
   Git for Windows does contain git-credential-wincred
wincred: avoid overwriting configured variables
   Generally, overriding these variables this deep is evil.
   (We could use ?= if really necessary.)

Erik, does this iteration look better?

Stepan

 contrib/credential/wincred/Makefile | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
1.9.2.msysgit.0.161.g83227c1

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] wincred: add install target
  2014-05-13  5:59       ` [PATCH 0/2] wincred makefile changes Stepan Kasal
@ 2014-05-13  6:01         ` Stepan Kasal
  2014-05-13  6:01         ` [PATCH 2/2] wincred: avoid overwriting configured variables Stepan Kasal
  1 sibling, 0 replies; 12+ messages in thread
From: Stepan Kasal @ 2014-05-13  6:01 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: GIT Mailing-list, Pat Thoyts, msysgit

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>
---
 contrib/credential/wincred/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index bad45ca..39fa5e0 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -7,8 +7,16 @@ CFLAGS = -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
+prefix ?= /usr/local
+libexecdir ?= $(prefix)/libexec/git-core
+
+INSTALL ?= install
+
 git-credential-wincred.exe : git-credential-wincred.c
 	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
+install: git-credential-wincred.exe
+	$(INSTALL) -m 755 $^ $(libexecdir)
+
 clean:
 	$(RM) git-credential-wincred.exe
-- 
1.9.2.msysgit.0.161.g83227c1

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] wincred: avoid overwriting configured variables
  2014-05-13  5:59       ` [PATCH 0/2] wincred makefile changes Stepan Kasal
  2014-05-13  6:01         ` [PATCH 1/2] wincred: add install target Stepan Kasal
@ 2014-05-13  6:01         ` Stepan Kasal
  2014-05-13  6:34           ` Erik Faye-Lund
  1 sibling, 1 reply; 12+ messages in thread
From: Stepan Kasal @ 2014-05-13  6:01 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: GIT Mailing-list, Pat Thoyts, msysgit

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>
---
 contrib/credential/wincred/Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index 39fa5e0..e64cd9a 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,9 +1,5 @@
 all: git-credential-wincred.exe
 
-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
-
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-- 
1.9.2.msysgit.0.161.g83227c1

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] wincred: avoid overwriting configured variables
  2014-05-13  6:01         ` [PATCH 2/2] wincred: avoid overwriting configured variables Stepan Kasal
@ 2014-05-13  6:34           ` Erik Faye-Lund
  2014-05-13  6:53             ` Stepan Kasal
  2014-05-13 13:18             ` Felipe Contreras
  0 siblings, 2 replies; 12+ messages in thread
From: Erik Faye-Lund @ 2014-05-13  6:34 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, Pat Thoyts, msysGit

On Tue, May 13, 2014 at 8:01 AM, Stepan Kasal <kasal@ucw.cz> wrote:
> 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>
> ---
>  contrib/credential/wincred/Makefile | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
> index 39fa5e0..e64cd9a 100644
> --- a/contrib/credential/wincred/Makefile
> +++ b/contrib/credential/wincred/Makefile
> @@ -1,9 +1,5 @@
>  all: git-credential-wincred.exe
>
> -CC = gcc
> -RM = rm -f
> -CFLAGS = -O2 -Wall
> -

Would it be better to set these if not already set, i.e:

-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -O2 -Wall

instead?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] wincred: avoid overwriting configured variables
  2014-05-13  6:34           ` Erik Faye-Lund
@ 2014-05-13  6:53             ` Stepan Kasal
       [not found]               ` <CABPQNSbb-rdTjCDYoC7uApfcMH8Q0Y-w4Tm9UiN6PhEeD-Gv6Q@mail.gmail.com>
  2014-05-13 13:18             ` Felipe Contreras
  1 sibling, 1 reply; 12+ messages in thread
From: Stepan Kasal @ 2014-05-13  6:53 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: GIT Mailing-list, Pat Thoyts, msysGit

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>
---

Hi kusma,

On Tue, May 13, 2014 at 08:34:36AM +0200, Erik Faye-Lund wrote:
> Would it be better to set these if not already set, i.e:
> 
> -CC = gcc
> -RM = rm -f
> -CFLAGS = -O2 -Wall
> +CC ?= gcc
> +RM ?= rm -f
> +CFLAGS ?= -O2 -Wall
> 
> instead?

... and moving it after the two includes, so that it does not
override the values set there?

Can you ack this?

 contrib/credential/wincred/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index 39fa5e0..6e992c0 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,12 +1,12 @@
 all: git-credential-wincred.exe
 
-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
-
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -O2 -Wall
+
 prefix ?= /usr/local
 libexecdir ?= $(prefix)/libexec/git-core
 
-- 
1.9.2.msysgit.0.161.g83227c1

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] wincred: avoid overwriting configured variables
  2014-05-13  6:34           ` Erik Faye-Lund
  2014-05-13  6:53             ` Stepan Kasal
@ 2014-05-13 13:18             ` Felipe Contreras
  2014-05-13 13:53               ` Stepan Kasal
  1 sibling, 1 reply; 12+ messages in thread
From: Felipe Contreras @ 2014-05-13 13:18 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: Stepan Kasal, GIT Mailing-list, Pat Thoyts, msysGit

On Tue, May 13, 2014 at 1:34 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Tue, May 13, 2014 at 8:01 AM, Stepan Kasal <kasal@ucw.cz> wrote:
>> 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>
>> ---
>>  contrib/credential/wincred/Makefile | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
>> index 39fa5e0..e64cd9a 100644
>> --- a/contrib/credential/wincred/Makefile
>> +++ b/contrib/credential/wincred/Makefile
>> @@ -1,9 +1,5 @@
>>  all: git-credential-wincred.exe
>>
>> -CC = gcc
>> -RM = rm -f
>> -CFLAGS = -O2 -Wall
>> -
>
> Would it be better to set these if not already set, i.e:
>
> -CC = gcc
> -RM = rm -f
> -CFLAGS = -O2 -Wall
> +CC ?= gcc
> +RM ?= rm -f
> +CFLAGS ?= -O2 -Wall
>
> instead?

Set by whom? If it's by the environment you should be running 'make -e'.

-- 
Felipe Contreras

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] wincred: avoid overwriting configured variables
  2014-05-13 13:18             ` Felipe Contreras
@ 2014-05-13 13:53               ` Stepan Kasal
  0 siblings, 0 replies; 12+ messages in thread
From: Stepan Kasal @ 2014-05-13 13:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Erik Faye-Lund, GIT Mailing-list, Pat Thoyts, msysGit

Hello,

On Tue, May 13, 2014 at 08:18:26AM -0500, Felipe Contreras wrote:
> Set by whom? If it's by the environment you should be running 'make -e'.

... or on command line (through recursive make, prehaps).
But these both take precedence over makefile assignments.

Another issue is the interaction with included makefile fragments.
Actually, both
	CC = gcc
	-include ../../../config.mak
and
	-include ../../../config.mak
	CC ?= gcc
are very close.  (They would differ if config.mak contained "?=".)

I was confused by the former, but perhaps it's only me.

Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] wincred: avoid overwriting configured variables
       [not found]                 ` <20140514084509.GA3134@camelia.ucw.cz>
@ 2014-05-14  8:49                   ` Erik Faye-Lund
  0 siblings, 0 replies; 12+ messages in thread
From: Erik Faye-Lund @ 2014-05-14  8:49 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, Pat Thoyts, msysGit

On Wed, May 14, 2014 at 10:45 AM, Stepan Kasal <kasal@ucw.cz> wrote:
> Hello kusma,
>
> On Tue, May 13, 2014 at 08:56:54AM +0200, Erik Faye-Lund wrote:
>> > --- a/contrib/credential/wincred/Makefile
>> > +++ b/contrib/credential/wincred/Makefile
>> > @@ -1,12 +1,12 @@
>> >  all: git-credential-wincred.exe
>> >
>> > -CC = gcc
>> > -RM = rm -f
>> > -CFLAGS = -O2 -Wall
>> > -
>> >  -include ../../../config.mak.autogen
>> >  -include ../../../config.mak
>> >
>> > +CC ?= gcc
>> > +RM ?= rm -f
>> > +CFLAGS ?= -O2 -Wall
>> > +
>> >  prefix ?= /usr/local
>> >  libexecdir ?= $(prefix)/libexec/git-core
>> >
>>
>> Yeah, looks good to me.
>
> thanks, but it looks you replied only to my personal mail.  Was it
> intentional?

No, sorry about that.

Consider the patches

Acked-by: Erik Faye-Lund <kusmabite@gmail.com>

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-05-14  8:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-30  6:46 [PATCH] wincred: add install target and avoid overwriting configured variables Stepan Kasal
2014-04-30  9:21 ` Erik Faye-Lund
2014-04-30 11:27   ` Stepan Kasal
2014-04-30 11:56     ` Erik Faye-Lund
2014-05-13  5:59       ` [PATCH 0/2] wincred makefile changes Stepan Kasal
2014-05-13  6:01         ` [PATCH 1/2] wincred: add install target Stepan Kasal
2014-05-13  6:01         ` [PATCH 2/2] wincred: avoid overwriting configured variables Stepan Kasal
2014-05-13  6:34           ` Erik Faye-Lund
2014-05-13  6:53             ` Stepan Kasal
     [not found]               ` <CABPQNSbb-rdTjCDYoC7uApfcMH8Q0Y-w4Tm9UiN6PhEeD-Gv6Q@mail.gmail.com>
     [not found]                 ` <20140514084509.GA3134@camelia.ucw.cz>
2014-05-14  8:49                   ` Erik Faye-Lund
2014-05-13 13:18             ` Felipe Contreras
2014-05-13 13:53               ` Stepan Kasal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).