[PATCH trace-cmd 1/3] parse-events: Add support for printing short fields.

[PATCH trace-cmd 1/3] parse-events: Add support for printing short fields.

[David Sharp]

Handle "%hd" etc. in pretty_print()

Signed-off-by: David Sharp <dhsharp@google.com>
Google-Bug-Id: 3501052
---
 parse-events.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 3d59d92..bfb7ff5 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -3607,6 +3607,9 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 			case '#':
 				/* FIXME: need to handle properly */
 				goto cont_process;
+			case 'h':
+				ls--;
+				goto cont_process;
 			case 'l':
 				ls++;
 				goto cont_process;
@@ -3687,6 +3690,18 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 						strcpy(format, "0x%llx");
 				}
 				switch (ls) {
+				case -2:
+					if (len_as_arg)
+						trace_seq_printf(s, format, len_arg, (char)val);
+					else
+						trace_seq_printf(s, format, (char)val);
+					break;
+				case -1:
+					if (len_as_arg)
+						trace_seq_printf(s, format, len_arg, (short)val);
+					else
+						trace_seq_printf(s, format, (short)val);
+					break;
 				case 0:
 					if (len_as_arg)
 						trace_seq_printf(s, format, len_arg, (int)val);
-- 
1.7.3.1

[PATCH trace-cmd 2/3] parse-events: support additional operators: '!', '~', and '!='

[David Sharp]

Add support for the unary operators '!' and '~', and support '!=' so that
it is differentiated from '!'.

Signed-off-by: David Sharp <dhsharp@google.com>
---
 parse-events.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index bfb7ff5..507621b 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -1564,6 +1564,9 @@ static int get_op_prio(char *op)
 {
 	if (!op[1]) {
 		switch (op[0]) {
+		case '~':
+		case '!':
+			return 4;
 		case '*':
 		case '/':
 		case '%':
@@ -1642,6 +1645,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 			goto out_free;
 		}
 		switch (token[0]) {
+		case '~':
 		case '!':
 		case '+':
 		case '-':
@@ -2981,6 +2985,21 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
 		left = eval_num_arg(data, size, event, arg->op.left);
 		right = eval_num_arg(data, size, event, arg->op.right);
 		switch (arg->op.op[0]) {
+		case '!':
+			switch (arg->op.op[1]) {
+			case 0:
+				val = !right;
+				break;
+			case '=':
+				val = left != right;
+				break;
+			default:
+				die("unknown op '%s'", arg->op.op);
+			}
+			break;
+		case '~':
+			val = ~right;
+			break;
 		case '|':
 			if (arg->op.op[1])
 				val = left || right;
-- 
1.7.3.1

[PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[David Sharp]

This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.

Make has default values CC and AR of 'cc' and 'ar' respectively. This means
that "CC ?= anything" will never have effect, because CC is always already set.
Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
environment useless.

Signed-off-by: David Sharp <dhsharp@google.com>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 169fcbc..fa37df5 100644
--- a/Makefile
+++ b/Makefile
@@ -13,8 +13,8 @@ FILE_VERSION = 6
 
 MAKEFLAGS += --no-print-directory
 
-CC ?= $(CROSS_COMPILE)gcc
-AR ?= $(CROSS_COMPILE)ar
+CC = $(CROSS_COMPILE)gcc
+AR = $(CROSS_COMPILE)ar
 EXT = -std=gnu99
 INSTALL = install
 
-- 
1.7.3.1

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 15:58 -0800, David Sharp wrote:
> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
> 
> Make has default values CC and AR of 'cc' and 'ar' respectively. This means
> that "CC ?= anything" will never have effect, because CC is always already set.
> Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
> environment useless.

Darren, can you verify this, as you were the one to make the original
change. I never had to cross compile it, I always did it natively.

David, Thanks! I'll go ahead and apply patch 1 and 2, and I'll wait for
a reply from Darren for this patch.

-- Steve

> 
> Signed-off-by: David Sharp <dhsharp@google.com>
> Cc: Darren Hart <dvhart@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> ---
>  Makefile |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 169fcbc..fa37df5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -13,8 +13,8 @@ FILE_VERSION = 6
>  
>  MAKEFLAGS += --no-print-directory
>  
> -CC ?= $(CROSS_COMPILE)gcc
> -AR ?= $(CROSS_COMPILE)ar
> +CC = $(CROSS_COMPILE)gcc
> +AR = $(CROSS_COMPILE)ar
>  EXT = -std=gnu99
>  INSTALL = install
>  

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 20:21 -0500, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 15:58 -0800, David Sharp wrote:
> > This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
> > 
> > Make has default values CC and AR of 'cc' and 'ar' respectively. This means
> > that "CC ?= anything" will never have effect, because CC is always already set.
> > Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
> > environment useless.
> 
> Darren, can you verify this, as you were the one to make the original
> change. I never had to cross compile it, I always did it natively.

OK, I just proved that David is correct, with the following make file:

---
CC ?= foo
AR ?= bar
all:
	echo what is $(CC) $(AR)
---

$ make
what is cc ar


Darren, can you just give an Acked-by anyway. I hate to apply a revert
of your patch without you doing so.

Thanks,

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 05:28 PM, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 20:21 -0500, Steven Rostedt wrote:
>> On Wed, 2011-03-09 at 15:58 -0800, David Sharp wrote:
>>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>>>
>>> Make has default values CC and AR of 'cc' and 'ar' respectively. This means
>>> that "CC ?= anything" will never have effect, because CC is always already set.
>>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
>>> environment useless.
>>
>> Darren, can you verify this, as you were the one to make the original
>> change. I never had to cross compile it, I always did it natively.
>
> OK, I just proved that David is correct, with the following make file:
>
> ---
> CC ?= foo
> AR ?= bar
> all:
> 	echo what is $(CC) $(AR)

> ---
>
> $ make
> what is cc ar
>
>
> Darren, can you just give an Acked-by anyway. I hate to apply a revert
> of your patch without you doing so.

I really thought I tested that - clearly not :/ Sorry about that. Please 
see my response to David for an alternate proposal.

--
Darren

>
> Thanks,
>
> -- Steve
>
>


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 03:58 PM, David Sharp wrote:
> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>
> Make has default values CC and AR of 'cc' and 'ar' respectively. This means
> that "CC ?= anything" will never have effect, because CC is always already set.
> Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
> environment useless.

The problem with this approach is it prevents the user from setting CC 
explicitly with the environment which is a very common way of using a 
specific version of gcc (for example). It also places restrictions on 
the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot 
work), this isn't acceptable.

You could use CC=your-cross-compiler, and if that doesn't work for you, 
you could prepare a patch that conditionally sets CC only if 
CROSS_COMPILE is set, but please do not simply revert this patch which 
solved a real problem with the Makefile.

--
Darren

> Signed-off-by: David Sharp<dhsharp@google.com>
> Cc: Darren Hart<dvhart@linux.intel.com>
> Cc: Steven Rostedt<rostedt@goodmis.org>
> ---
>   Makefile |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 169fcbc..fa37df5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -13,8 +13,8 @@ FILE_VERSION = 6
>
>   MAKEFLAGS += --no-print-directory
>
> -CC ?= $(CROSS_COMPILE)gcc
> -AR ?= $(CROSS_COMPILE)ar
> +CC = $(CROSS_COMPILE)gcc
> +AR = $(CROSS_COMPILE)ar
>   EXT = -std=gnu99
>   INSTALL = install
>


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
> On 03/09/2011 03:58 PM, David Sharp wrote:
> > This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
> >
> > Make has default values CC and AR of 'cc' and 'ar' respectively. This means
> > that "CC ?= anything" will never have effect, because CC is always already set.
> > Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
> > environment useless.
> 
> The problem with this approach is it prevents the user from setting CC 
> explicitly with the environment which is a very common way of using a 
> specific version of gcc (for example). It also places restrictions on 
> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot 
> work), this isn't acceptable.
> 
> You could use CC=your-cross-compiler, and if that doesn't work for you, 
> you could prepare a patch that conditionally sets CC only if 
> CROSS_COMPILE is set, but please do not simply revert this patch which 
> solved a real problem with the Makefile.

Hmm, but the thing is, the change did not work, unless your environment
for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
compiler that you wanted, where 'gcc' would not.

Thus, would you be fine with something like:

	BUILD_CC ?=	$(CROSS_COMPILE)gcc
	CC	 =	$(BUILD_CC)

Then you could just update BUILD_CC and that will update CC for you.

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 05:36 PM, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
>> On 03/09/2011 03:58 PM, David Sharp wrote:
>>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>>>
>>> Make has default values CC and AR of 'cc' and 'ar' respectively. This means
>>> that "CC ?= anything" will never have effect, because CC is always already set.
>>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
>>> environment useless.
>>
>> The problem with this approach is it prevents the user from setting CC
>> explicitly with the environment which is a very common way of using a
>> specific version of gcc (for example). It also places restrictions on
>> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot
>> work), this isn't acceptable.
>>
>> You could use CC=your-cross-compiler, and if that doesn't work for you,
>> you could prepare a patch that conditionally sets CC only if
>> CROSS_COMPILE is set, but please do not simply revert this patch which
>> solved a real problem with the Makefile.
>
> Hmm, but the thing is, the change did not work,

It did work for me as I was setting CC= on the command line.

unless your environment
> for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
> compiler that you wanted, where 'gcc' would not.
>
> Thus, would you be fine with something like:
>
> 	BUILD_CC ?=	$(CROSS_COMPILE)gcc
> 	CC	 =	$(BUILD_CC)

This would also work, but what is wrong with:

dvhart@doubt:templates$ cat Makefile
ifdef CROSS_COMPILE
CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar
endif

all:
	echo "CC: $(CC)"

dvhart@doubt:templates$ make -s
CC: cc

dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
CC: gcc-4.5.1

dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
CC: my-cross-gcc


Seems to meet everyone's needs without changing any tools/scripts/etc 
that have used trace-cmd before or after the CC ?= wreckage.



Thanks,

--
Darren

>
> Then you could just update BUILD_CC and that will update CC for you.
>
> -- Steve
>
>


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[David Sharp]

On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart <dvhart@linux.intel.com> wrote:
> On 03/09/2011 05:36 PM, Steven Rostedt wrote:
>> On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
>>> On 03/09/2011 03:58 PM, David Sharp wrote:
>>>>
>>>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>>>>
>>>> Make has default values CC and AR of 'cc' and 'ar' respectively. This
>>>> means
>>>> that "CC ?= anything" will never have effect, because CC is always
>>>> already set.
>>>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command
>>>> line or
>>>> environment useless.
>>>
>>> The problem with this approach is it prevents the user from setting CC
>>> explicitly with the environment which is a very common way of using a
>>> specific version of gcc (for example). It also places restrictions on
>>> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot
>>> work), this isn't acceptable.
>>>
>>> You could use CC=your-cross-compiler, and if that doesn't work for you,
>>> you could prepare a patch that conditionally sets CC only if
>>> CROSS_COMPILE is set, but please do not simply revert this patch which
>>> solved a real problem with the Makefile.
>>
>> Hmm, but the thing is, the change did not work,
>
> It did work for me as I was setting CC= on the command line.
>
> unless your environment
>>
>> for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
>> compiler that you wanted, where 'gcc' would not.
>>
>> Thus, would you be fine with something like:
>>
>>        BUILD_CC ?=     $(CROSS_COMPILE)gcc
>>        CC       =      $(BUILD_CC)
>
> This would also work, but what is wrong with:
>
> dvhart@doubt:templates$ cat Makefile
> ifdef CROSS_COMPILE
> CC = $(CROSS_COMPILE)gcc
> AR = $(CROSS_COMPILE)ar
> endif
>
> all:
>        echo "CC: $(CC)"
>
> dvhart@doubt:templates$ make -s
> CC: cc
>
> dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
> CC: gcc-4.5.1
>
> dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
> CC: my-cross-gcc
>
>
> Seems to meet everyone's needs without changing any tools/scripts/etc that
> have used trace-cmd before or after the CC ?= wreckage.

It's a little odd that the default CC is "cc" unless you supply
CROSS_COMPILE, then it's "gcc". I'd probably be okay with this, but I
would think it's weird.

I don't know the answers, but if we take the kernel Makefile as a
template, then setting CC doesn't work.

>
>
>
> Thanks,
>
> --
> Darren
>
>>
>> Then you could just update BUILD_CC and that will update CC for you.
>>
>> -- Steve
>>
>>
>
>
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel
>

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 18:27 -0800, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart <dvhart@linux.intel.com> wrote:

> > dvhart@doubt:templates$ cat Makefile
> > ifdef CROSS_COMPILE
> > CC = $(CROSS_COMPILE)gcc
> > AR = $(CROSS_COMPILE)ar
> > endif
> >
> > all:
> >        echo "CC: $(CC)"
> >
> > dvhart@doubt:templates$ make -s
> > CC: cc
> >
> > dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
> > CC: gcc-4.5.1
> >
> > dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
> > CC: my-cross-gcc
> >
> >
> > Seems to meet everyone's needs without changing any tools/scripts/etc that
> > have used trace-cmd before or after the CC ?= wreckage.
> 
> It's a little odd that the default CC is "cc" unless you supply
> CROSS_COMPILE, then it's "gcc". I'd probably be okay with this, but I
> would think it's weird.
> 
> I don't know the answers, but if we take the kernel Makefile as a
> template, then setting CC doesn't work.
> 

I really don't care much for this either. But I'm trying to make it work
for everyone. Honestly, I think the BUILD_CC version is the cleanest,
but I understand that this will add a burden onto Darren to fix his
tools to handle it, whereas, I would like to avoid that.

I'll play with some other make tricks and see if I can come up with a
better solution.

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 21:51 -0500, Steven Rostedt wrote:

> I'll play with some other make tricks and see if I can come up with a
> better solution.

OK, it didn't take me long to come up with "Makefiles suck" ;)

But I did come up with a solution:

ifneq ("$(origin CC)", "environment")
CC = gcc
endif

CC := $(CROSS_COMPILE)$(CC)

This wont let make CC=xx work unless I also add a:

ifneq ("$(origin CC)", "command line")

around the above if, but do we care?

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[David Sharp]

On Wed, Mar 9, 2011 at 7:26 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2011-03-09 at 21:51 -0500, Steven Rostedt wrote:
>
>> I'll play with some other make tricks and see if I can come up with a
>> better solution.
>
> OK, it didn't take me long to come up with "Makefiles suck" ;)

Yes, except for very basic things.

>
> But I did come up with a solution:
>
> ifneq ("$(origin CC)", "environment")
> CC = gcc
> endif
>
> CC := $(CROSS_COMPILE)$(CC)
>
> This wont let make CC=xx work unless I also add a:
>
> ifneq ("$(origin CC)", "command line")
>
> around the above if, but do we care?
>
> -- Steve

This seems to do it all:

define allow-override
  $(if $(or $(findstring environment,$(origin $(1))),
            $(findstring command line,$(origin $(1)))),,\
    $(eval $(1) = $(2)))
endef

$(call allow-override,CC,$(CROSS_COMPILE)gcc)
$(call allow-override,AR,$(CROSS_COMPILE)ar)

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 09:25 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 7:26 PM, Steven Rostedt<rostedt@goodmis.org>  wrote:
>> On Wed, 2011-03-09 at 21:51 -0500, Steven Rostedt wrote:
>>
>>> I'll play with some other make tricks and see if I can come up with a
>>> better solution.
>>
>> OK, it didn't take me long to come up with "Makefiles suck" ;)
>
> Yes, except for very basic things.
>
>>
>> But I did come up with a solution:
>>
>> ifneq ("$(origin CC)", "environment")
>> CC = gcc
>> endif
>>
>> CC := $(CROSS_COMPILE)$(CC)
>>
>> This wont let make CC=xx work unless I also add a:
>>
>> ifneq ("$(origin CC)", "command line")
>>
>> around the above if, but do we care?
>>
>> -- Steve
>
> This seems to do it all:
>
> define allow-override
>    $(if $(or $(findstring environment,$(origin $(1))),
>              $(findstring command line,$(origin $(1)))),,\
>      $(eval $(1) = $(2)))
> endef
>
> $(call allow-override,CC,$(CROSS_COMPILE)gcc)
> $(call allow-override,AR,$(CROSS_COMPILE)ar)

Egads .... that's hideous :-) This level of complexity makes it very 
difficult for people to readily understand it. What does this offer over:

ifdef CROSS_COMPILE
CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar
endif

besides being 3 lines longer with much more complex Makefile syntax and 
conditional statements?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 22:46 -0800, Darren Hart wrote:

> > This seems to do it all:
> >
> > define allow-override
> >    $(if $(or $(findstring environment,$(origin $(1))),
> >              $(findstring command line,$(origin $(1)))),,\
> >      $(eval $(1) = $(2)))
> > endef
> >
> > $(call allow-override,CC,$(CROSS_COMPILE)gcc)
> > $(call allow-override,AR,$(CROSS_COMPILE)ar)
> 
> Egads .... that's hideous :-) This level of complexity makes it very 
> difficult for people to readily understand it. What does this offer over:

Love the world of makefiles ;) Nothing that comments wont solve.

> 
> ifdef CROSS_COMPILE
> CC = $(CROSS_COMPILE)gcc
> AR = $(CROSS_COMPILE)ar
> endif
> 
> besides being 3 lines longer with much more complex Makefile syntax and 
> conditional statements?

Yes, the above is simple and solves the issue for both you and David,
but it has a side-effect that David already pointed out. CC is now cc
and not gcc. I don't like that, as I do have systems that cc is
different that gcc and I want to use gcc.

But if I define CROSS_COMPILE it then suddenly uses gcc. Yes this may
not seem like a big deal now, but in the future, it could cause a lot of
headache. I rather have the more complex yet correct solution that is
consistent than a simple easy to read solution with a subtle side-effect
that is hidden.

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 07:26 PM, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 21:51 -0500, Steven Rostedt wrote:
>
>> I'll play with some other make tricks and see if I can come up with a
>> better solution.
>
> OK, it didn't take me long to come up with "Makefiles suck" ;)

Yeah, that little tidbit I sent took longer to come up with than it 
should have :/

>
> But I did come up with a solution:
>
> ifneq ("$(origin CC)", "environment")
> CC = gcc
> endif
>
> CC := $(CROSS_COMPILE)$(CC)

Why do we want to force CC=gcc? Isn't the right thing to make your OS 
setup cc to point to your preferred compiler since it is known to be the 
default for make?

On Ubuntu, cc -> gcc
On Fedora 13, cc -> ccache

seems strange to force it to be gcc when users/distros have gone through 
the trouble to set it up on their system.


> This wont let make CC=xx work unless I also add a:
>
> ifneq ("$(origin CC)", "command line")
>
> around the above if, but do we care?
>

This starts to get to the point where others looking at it will choke on 
the expert Makefile usage.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 22:41 -0800, Darren Hart wrote:
> On 03/09/2011 07:26 PM, Steven Rostedt wrote:
> > On Wed, 2011-03-09 at 21:51 -0500, Steven Rostedt wrote:
> >
> >> I'll play with some other make tricks and see if I can come up with a
> >> better solution.
> >
> > OK, it didn't take me long to come up with "Makefiles suck" ;)
> 
> Yeah, that little tidbit I sent took longer to come up with than it 
> should have :/
> 
> >
> > But I did come up with a solution:
> >
> > ifneq ("$(origin CC)", "environment")
> > CC = gcc
> > endif
> >
> > CC := $(CROSS_COMPILE)$(CC)
> 
> Why do we want to force CC=gcc? Isn't the right thing to make your OS 
> setup cc to point to your preferred compiler since it is known to be the 
> default for make?

Why not? The kernel does it. And yes, as I am following the kernel with
trace-cmd than other user space tools.

> 
> On Ubuntu, cc -> gcc
> On Fedora 13, cc -> ccache

Ug, thanks for telling me. /me goes to disable ccache from his F13
installs.

> 
> seems strange to force it to be gcc when users/distros have gone through 
> the trouble to set it up on their system.

If you define CC as a environment variable, it will work with the other
solution.

> 
> 
> > This wont let make CC=xx work unless I also add a:
> >
> > ifneq ("$(origin CC)", "command line")
> >
> > around the above if, but do we care?
> >
> 
> This starts to get to the point where others looking at it will choke on 
> the expert Makefile usage.
> 

Yeah, that took a bit of searching. Makefiles are far from being
obvious. But as you all know, I work in the world of obfuscation.
Probably why I prefer perl over python ;)

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 06:51 PM, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 18:27 -0800, David Sharp wrote:
>> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart<dvhart@linux.intel.com>  wrote:
>
>>> dvhart@doubt:templates$ cat Makefile
>>> ifdef CROSS_COMPILE
>>> CC = $(CROSS_COMPILE)gcc
>>> AR = $(CROSS_COMPILE)ar
>>> endif
>>>
>>> all:
>>>         echo "CC: $(CC)"
>>>
>>> dvhart@doubt:templates$ make -s
>>> CC: cc
>>>
>>> dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
>>> CC: gcc-4.5.1
>>>
>>> dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
>>> CC: my-cross-gcc
>>>
>>>
>>> Seems to meet everyone's needs without changing any tools/scripts/etc that
>>> have used trace-cmd before or after the CC ?= wreckage.
>>
>> It's a little odd that the default CC is "cc" unless you supply
>> CROSS_COMPILE, then it's "gcc". I'd probably be okay with this, but I
>> would think it's weird.
>>
>> I don't know the answers, but if we take the kernel Makefile as a
>> template, then setting CC doesn't work.
>>
>
> I really don't care much for this either. But I'm trying to make it work
> for everyone. Honestly, I think the BUILD_CC version is the cleanest,
> but I understand that this will add a burden onto Darren to fix his
> tools to handle it, whereas, I would like to avoid that.

This is a very minor issue and will take me less time to fix than 
another half-dozen emails arguing for a different solution :-) However, 
being able to specify CC on the command line _is_ a very common thing, 
and preventing it from working will likely cause this to come up again 
in the future.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 06:27 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart<dvhart@linux.intel.com>  wrote:
>> On 03/09/2011 05:36 PM, Steven Rostedt wrote:
>>> On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
>>>> On 03/09/2011 03:58 PM, David Sharp wrote:
>>>>>
>>>>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>>>>>
>>>>> Make has default values CC and AR of 'cc' and 'ar' respectively. This
>>>>> means
>>>>> that "CC ?= anything" will never have effect, because CC is always
>>>>> already set.
>>>>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command
>>>>> line or
>>>>> environment useless.
>>>>
>>>> The problem with this approach is it prevents the user from setting CC
>>>> explicitly with the environment which is a very common way of using a
>>>> specific version of gcc (for example). It also places restrictions on
>>>> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot
>>>> work), this isn't acceptable.
>>>>
>>>> You could use CC=your-cross-compiler, and if that doesn't work for you,
>>>> you could prepare a patch that conditionally sets CC only if
>>>> CROSS_COMPILE is set, but please do not simply revert this patch which
>>>> solved a real problem with the Makefile.
>>>
>>> Hmm, but the thing is, the change did not work,
>>
>> It did work for me as I was setting CC= on the command line.
>>
>> unless your environment
>>>
>>> for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
>>> compiler that you wanted, where 'gcc' would not.
>>>
>>> Thus, would you be fine with something like:
>>>
>>>         BUILD_CC ?=     $(CROSS_COMPILE)gcc
>>>         CC       =      $(BUILD_CC)
>>
>> This would also work, but what is wrong with:
>>
>> dvhart@doubt:templates$ cat Makefile
>> ifdef CROSS_COMPILE
>> CC = $(CROSS_COMPILE)gcc
>> AR = $(CROSS_COMPILE)ar
>> endif
>>
>> all:
>>         echo "CC: $(CC)"
>>
>> dvhart@doubt:templates$ make -s
>> CC: cc
>>
>> dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
>> CC: gcc-4.5.1
>>
>> dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
>> CC: my-cross-gcc
>>
>>
>> Seems to meet everyone's needs without changing any tools/scripts/etc that
>> have used trace-cmd before or after the CC ?= wreckage.
>
> It's a little odd that the default CC is "cc" unless you supply
> CROSS_COMPILE, then it's "gcc". I'd probably be okay with this, but I
> would think it's weird.

Note that I don't specify cc anywhere, that is what Make uses as the 
default CC, and at least on Ubuntu 10.10 /usr/bin/cc is in the path and 
points to gcc (eventually). So this seems pretty no

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/09/2011 06:27 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart<dvhart@linux.intel.com>  wrote:
>> On 03/09/2011 05:36 PM, Steven Rostedt wrote:
>>> On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
>>>> On 03/09/2011 03:58 PM, David Sharp wrote:
>>>>>
>>>>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
>>>>>
>>>>> Make has default values CC and AR of 'cc' and 'ar' respectively. This
>>>>> means
>>>>> that "CC ?= anything" will never have effect, because CC is always
>>>>> already set.
>>>>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command
>>>>> line or
>>>>> environment useless.
>>>>
>>>> The problem with this approach is it prevents the user from setting CC
>>>> explicitly with the environment which is a very common way of using a
>>>> specific version of gcc (for example). It also places restrictions on
>>>> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot
>>>> work), this isn't acceptable.
>>>>
>>>> You could use CC=your-cross-compiler, and if that doesn't work for you,
>>>> you could prepare a patch that conditionally sets CC only if
>>>> CROSS_COMPILE is set, but please do not simply revert this patch which
>>>> solved a real problem with the Makefile.
>>>
>>> Hmm, but the thing is, the change did not work,
>>
>> It did work for me as I was setting CC= on the command line.
>>
>> unless your environment
>>>
>>> for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
>>> compiler that you wanted, where 'gcc' would not.
>>>
>>> Thus, would you be fine with something like:
>>>
>>>         BUILD_CC ?=     $(CROSS_COMPILE)gcc
>>>         CC       =      $(BUILD_CC)
>>
>> This would also work, but what is wrong with:
>>
>> dvhart@doubt:templates$ cat Makefile
>> ifdef CROSS_COMPILE
>> CC = $(CROSS_COMPILE)gcc
>> AR = $(CROSS_COMPILE)ar
>> endif
>>
>> all:
>>         echo "CC: $(CC)"
>>
>> dvhart@doubt:templates$ make -s
>> CC: cc
>>
>> dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
>> CC: gcc-4.5.1
>>
>> dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
>> CC: my-cross-gcc
>>
>>
>> Seems to meet everyone's needs without changing any tools/scripts/etc that
>> have used trace-cmd before or after the CC ?= wreckage.
>
> It's a little odd that the default CC is "cc" unless you supply
> CROSS_COMPILE, then it's "gcc". I'd probably be okay with this, but I
> would think it's weird.
>
> I don't know the answers, but if we take the kernel Makefile as a
> template, then setting CC doesn't work.

The kernel is a bit special I believe as it is pretty tied to gcc. Is 
trace-cmd tied to gcc to such a degree that we want to make it difficult 
for people to try different compilers?

--
Darren
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 22:43 -0800, Darren Hart wrote:

> > I don't know the answers, but if we take the kernel Makefile as a
> > template, then setting CC doesn't work.
> 
> The kernel is a bit special I believe as it is pretty tied to gcc. Is 
> trace-cmd tied to gcc to such a degree that we want to make it difficult 
> for people to try different compilers?

True, trace-cmd is tied more to glibc (special features) than gcc.

I just looked at the Makefile for evolution, and it has:

CC = gcc

So it is not limited to just the kernel.

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Darren Hart]

On 03/10/2011 05:11 AM, Steven Rostedt wrote:
> On Wed, 2011-03-09 at 22:43 -0800, Darren Hart wrote:
>
>>> I don't know the answers, but if we take the kernel Makefile as a
>>> template, then setting CC doesn't work.
>>
>> The kernel is a bit special I believe as it is pretty tied to gcc. Is
>> trace-cmd tied to gcc to such a degree that we want to make it difficult
>> for people to try different compilers?
>
> True, trace-cmd is tied more to glibc (special features) than gcc.
>
> I just looked at the Makefile for evolution, and it has:
>
> CC = gcc
>
> So it is not limited to just the kernel.

I think we're down to a subjective argument over "correctness". Any of 
the resent proposals will work for me.


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Thu, 2011-03-10 at 09:50 -0800, Darren Hart wrote:
> 
> On 03/10/2011 05:11 AM, Steven Rostedt wrote:
> > On Wed, 2011-03-09 at 22:43 -0800, Darren Hart wrote:
> >
> >>> I don't know the answers, but if we take the kernel Makefile as a
> >>> template, then setting CC doesn't work.
> >>
> >> The kernel is a bit special I believe as it is pretty tied to gcc. Is
> >> trace-cmd tied to gcc to such a degree that we want to make it difficult
> >> for people to try different compilers?
> >
> > True, trace-cmd is tied more to glibc (special features) than gcc.
> >
> > I just looked at the Makefile for evolution, and it has:
> >
> > CC = gcc
> >
> > So it is not limited to just the kernel.
> 
> I think we're down to a subjective argument over "correctness".

No no no don't stop! I love having subjective arguments over
"correctness"! ;)


>  Any of 
> the resent proposals will work for me.

OK, personally, I like the last one, even if it is an overkill of
Makefile obfuscation.

David, could you post that last change. I'll add a patch on top
commenting the reason for it, and how Makefile's suck ;)

-- Steve

[PATCH trace-cmd v2] trace-cmd: allow setting CC and AR, or CROSS_COMPILE from command line

[David Sharp]

Makefiles suck: Use some makefile magic to get the best of all worlds for
CC and AR: Sane defaults and the ability to override CC, and AR, or use
CROSS_COMPILE to set a prefix.

Signed-off-by: David Sharp <dhsharp@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Darren Hart <dvhart@linux.intel.com>
---
 Makefile |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 169fcbc..c4cd926 100644
--- a/Makefile
+++ b/Makefile
@@ -13,8 +13,22 @@ FILE_VERSION = 6
 
 MAKEFLAGS += --no-print-directory
 
-CC ?= $(CROSS_COMPILE)gcc
-AR ?= $(CROSS_COMPILE)ar
+
+# Makefiles suck: This macro sets a default value of $(2) for the
+# variable named by $(1), unless the variable has been set by
+# environment or command line. This is necessary for CC and AR
+# because make sets default values, so the simpler ?= approach
+# won't work as expected.
+define allow-override
+  $(if $(or $(findstring environment,$(origin $(1))),\
+            $(findstring command line,$(origin $(1)))),,\
+    $(eval $(1) = $(2)))
+endef
+
+# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
+$(call allow-override,CC,$(CROSS_COMPILE)gcc)
+$(call allow-override,AR,$(CROSS_COMPILE)ar)
+
 EXT = -std=gnu99
 INSTALL = install
 
-- 
1.7.3.1

Re: [PATCH trace-cmd v2] trace-cmd: allow setting CC and AR, or CROSS_COMPILE from command line

[Steven Rostedt]

On Thu, 2011-03-10 at 13:11 -0800, David Sharp wrote:
> Makefiles suck: Use some makefile magic to get the best of all worlds for
> CC and AR: Sane defaults and the ability to override CC, and AR, or use
> CROSS_COMPILE to set a prefix.
> 
> Signed-off-by: David Sharp <dhsharp@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Darren Hart <dvhart@linux.intel.com>
> ---

Applied, thanks David!

-- Steve

Re: [PATCH trace-cmd 3/3] Revert "trace-cmd: Use conditional assignment of CC and AR"

[Steven Rostedt]

On Wed, 2011-03-09 at 17:58 -0800, Darren Hart wrote:
> On 03/09/2011 05:36 PM, Steven Rostedt wrote:
> > On Wed, 2011-03-09 at 17:27 -0800, Darren Hart wrote:
> >> On 03/09/2011 03:58 PM, David Sharp wrote:
> >>> This reverts commit 6c696cec3f264a9399241b6e648f58bc97117d49.
> >>>
> >>> Make has default values CC and AR of 'cc' and 'ar' respectively. This means
> >>> that "CC ?= anything" will never have effect, because CC is always already set.
> >>> Because of this, 6c696cec makes setting CROSS_COMPILE from the command line or
> >>> environment useless.
> >>
> >> The problem with this approach is it prevents the user from setting CC
> >> explicitly with the environment which is a very common way of using a
> >> specific version of gcc (for example). It also places restrictions on
> >> the filename of the compiler (it must end in gcc - so gcc-4.5.1 cannot
> >> work), this isn't acceptable.
> >>
> >> You could use CC=your-cross-compiler, and if that doesn't work for you,
> >> you could prepare a patch that conditionally sets CC only if
> >> CROSS_COMPILE is set, but please do not simply revert this patch which
> >> solved a real problem with the Makefile.
> >
> > Hmm, but the thing is, the change did not work,
> 
> It did work for me as I was setting CC= on the command line.
> 
> unless your environment
> > for some reason does not supply a 'cc'. Or that 'cc' defaulted to the
> > compiler that you wanted, where 'gcc' would not.
> >
> > Thus, would you be fine with something like:
> >
> > 	BUILD_CC ?=	$(CROSS_COMPILE)gcc
> > 	CC	 =	$(BUILD_CC)
> 
> This would also work, but what is wrong with:
> 
> dvhart@doubt:templates$ cat Makefile
> ifdef CROSS_COMPILE
> CC = $(CROSS_COMPILE)gcc
> AR = $(CROSS_COMPILE)ar
> endif
> 
> all:
> 	echo "CC: $(CC)"
> 
> dvhart@doubt:templates$ make -s
> CC: cc
> 
> dvhart@doubt:templates$ CC=gcc-4.5.1 make -s
> CC: gcc-4.5.1
> 
> dvhart@doubt:templates$ CROSS_COMPILE=my-cross- make -s
> CC: my-cross-gcc
> 
> 
> Seems to meet everyone's needs without changing any tools/scripts/etc 
> that have used trace-cmd before or after the CC ?= wreckage.
> 

OK, I'm fine with this.

David, want to send another patch with Darren's suggestion?

-- Steve