2011-03-10 00:06:01

by David Sharp

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

Handle "%hd" etc. in pretty_print()

Signed-off-by: David Sharp <[email protected]>
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


2011-03-09 23:59:13

by David Sharp

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

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

Signed-off-by: David Sharp <[email protected]>
---
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

2011-03-10 00:29:20

by David Sharp

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

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 <[email protected]>
Cc: Darren Hart <[email protected]>
Cc: Steven Rostedt <[email protected]>
---
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

2011-03-10 01:21:48

by Steven Rostedt

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

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 <[email protected]>
> Cc: Darren Hart <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> ---
> 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
>

2011-03-10 01:27:25

by Darren Hart

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

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<[email protected]>
> Cc: Darren Hart<[email protected]>
> Cc: Steven Rostedt<[email protected]>
> ---
> 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

2011-03-10 01:28:12

by Steven Rostedt

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

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

2011-03-10 01:29:46

by Darren Hart

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

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

2011-03-10 01:36:43

by Steven Rostedt

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

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

2011-03-10 01:58:20

by Darren Hart

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

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

2011-03-10 02:33:21

by David Sharp

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

On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart <[email protected]> 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
>

2011-03-10 02:42:22

by Steven Rostedt

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

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

2011-03-10 02:51:30

by Steven Rostedt

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

On Wed, 2011-03-09 at 18:27 -0800, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart <[email protected]> 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

2011-03-10 03:26:37

by Steven Rostedt

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

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

2011-03-10 05:25:41

by David Sharp

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

On Wed, Mar 9, 2011 at 7:26 PM, Steven Rostedt <[email protected]> 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)

2011-03-10 06:32:39

by Darren Hart

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

On 03/09/2011 06:27 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart<[email protected]> 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

2011-03-10 06:34:43

by Darren Hart

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

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<[email protected]> 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

2011-03-10 06:41:28

by Darren Hart

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

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

2011-03-10 06:43:33

by Darren Hart

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

On 03/09/2011 06:27 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 5:58 PM, Darren Hart<[email protected]> 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

2011-03-10 06:46:17

by Darren Hart

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

On 03/09/2011 09:25 PM, David Sharp wrote:
> On Wed, Mar 9, 2011 at 7:26 PM, Steven Rostedt<[email protected]> 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

2011-03-10 13:02:45

by Steven Rostedt

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

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

2011-03-10 13:07:46

by Steven Rostedt

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

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

2011-03-10 13:11:12

by Steven Rostedt

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

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

2011-03-10 17:50:37

by Darren Hart

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



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

2011-03-10 18:11:08

by Steven Rostedt

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

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


2011-03-10 21:12:22

by David Sharp

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

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 <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Darren Hart <[email protected]>
---
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

2011-03-10 21:30:26

by Steven Rostedt

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

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 <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Darren Hart <[email protected]>
> ---

Applied, thanks David!

-- Steve