2010-08-16 12:41:04

by Kusanagi Kouichi

[permalink] [raw]
Subject: [PATCH] perf tools: Don't use brace expansion.

DASH doesn't support brace expansion.

Signed-off-by: Kusanagi Kouichi <[email protected]>
---
tools/perf/Makefile | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 41abb90..8fa851b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -157,8 +157,10 @@ all::
#
# Define NO_DWARF if you do not want debug-info analysis feature at all.

-$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
-$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
+$(shell sh -c 'mkdir -p $(OUTPUT)scripts/perl/Perf-Trace-Util/' 2> /dev/null)
+$(shell sh -c 'mkdir -p $(OUTPUT)scripts/python/Perf-Trace-Util/' 2> /dev/null)
+$(shell sh -c 'mkdir -p $(OUTPUT)util/ui/browsers/' 2> /dev/null)
+$(shell sh -c 'mkdir -p $(OUTPUT)util/scripting-engines/' 2> /dev/null)
$(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)

$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
--
1.7.1


2010-08-16 13:30:21

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 21:41 +0900, Kusanagi Kouichi wrote:
> DASH doesn't support brace expansion.

I really hate these second rate shells, but I guess we could do this on
the grounds of POSIX sh not specifying the brace expansion.

> Signed-off-by: Kusanagi Kouichi <[email protected]>
> ---
> tools/perf/Makefile | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 41abb90..8fa851b 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -157,8 +157,10 @@ all::
> #
> # Define NO_DWARF if you do not want debug-info analysis feature at all.
>
> -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/perl/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/python/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/ui/browsers/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/scripting-engines/' 2> /dev/null)
> $(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
>
> $(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE

2010-08-16 14:10:22

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 21:41 +0900, Kusanagi Kouichi wrote:
> DASH doesn't support brace expansion.
>
> Signed-off-by: Kusanagi Kouichi <[email protected]>
> ---
> tools/perf/Makefile | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 41abb90..8fa851b 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -157,8 +157,10 @@ all::
> #
> # Define NO_DWARF if you do not want debug-info analysis feature at all.
>
> -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/perl/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/python/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/ui/browsers/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/scripting-engines/' 2> /dev/null)
> $(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
>
> $(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE

The other solution is to use standard-make features like in
mkdir -p $(foreach d,ui/browsers scripting-engines,$(OUTPUT)util/$(d)/) 2> /dev/null

Is there actually a specific reason for the
$(shell sh -c '...')
around?
It looks superflous.

Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at

2010-08-16 14:31:00

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 16:09 +0200, Bernd Petrovitsch wrote:

> > -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> > -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)

> The other solution is to use standard-make features like in
> mkdir -p $(foreach d,ui/browsers scripting-engines,$(OUTPUT)util/$(d)/) 2> /dev/null
>
> Is there actually a specific reason for the
> $(shell sh -c '...')
> around?
> It looks superflous.

I think the reason is is that nobody who touched that file really knew
make all that well. Your version looks fine to me.

2010-08-16 14:55:09

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 16:30 +0200, Peter Zijlstra wrote:
> On Mon, 2010-08-16 at 16:09 +0200, Bernd Petrovitsch wrote:
>
> > > -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> > > -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
>
> > The other solution is to use standard-make features like in
> > mkdir -p $(foreach d,ui/browsers scripting-engines,$(OUTPUT)util/$(d)/) 2> /dev/null
> >
> > Is there actually a specific reason for the
> > $(shell sh -c '...')
> > around?
> > It looks superflous.
>
> I think the reason is is that nobody who touched that file really knew
> make all that well. Your version looks fine to me.

Ah, the reason is that they are not part of a rule but on the top-level
(and thus always executed).

Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at

2010-08-16 15:25:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

Em Mon, Aug 16, 2010 at 09:41:00PM +0900, Kusanagi Kouichi escreveu:
> DASH doesn't support brace expansion.

Ok, I guess this one is OK :-) Will merge.

- Arnaldo

> Signed-off-by: Kusanagi Kouichi <[email protected]>
> ---
> tools/perf/Makefile | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 41abb90..8fa851b 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -157,8 +157,10 @@ all::
> #
> # Define NO_DWARF if you do not want debug-info analysis feature at all.
>
> -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/perl/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)scripts/python/Perf-Trace-Util/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/ui/browsers/' 2> /dev/null)
> +$(shell sh -c 'mkdir -p $(OUTPUT)util/scripting-engines/' 2> /dev/null)
> $(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
>
> $(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
> --
> 1.7.1

2010-08-16 15:29:31

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

Em Mon, Aug 16, 2010 at 04:54:38PM +0200, Bernd Petrovitsch escreveu:
> On Mon, 2010-08-16 at 16:30 +0200, Peter Zijlstra wrote:
> > On Mon, 2010-08-16 at 16:09 +0200, Bernd Petrovitsch wrote:
> > > > -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> > > > -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)

> > > The other solution is to use standard-make features like in
> > > mkdir -p $(foreach d,ui/browsers scripting-engines,$(OUTPUT)util/$(d)/) 2> /dev/null

> > > Is there actually a specific reason for the
> > > $(shell sh -c '...')
> > > around?
> > > It looks superflous.

> > I think the reason is is that nobody who touched that file really knew
> > make all that well. Your version looks fine to me.

> Ah, the reason is that they are not part of a rule but on the top-level
> (and thus always executed).

So it worked by luck! /me runs :-P

More seriously, so there is a reason for that to be like that and you're
not aware of any other shorter or more convenient way of achieving that
goal, right?

- Arnaldo

2010-08-16 15:44:17

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 12:29 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 16, 2010 at 04:54:38PM +0200, Bernd Petrovitsch escreveu:
> > On Mon, 2010-08-16 at 16:30 +0200, Peter Zijlstra wrote:
> > > On Mon, 2010-08-16 at 16:09 +0200, Bernd Petrovitsch wrote:
> > > > > -$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
> > > > > -$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
>
> > > > The other solution is to use standard-make features like in
> > > > mkdir -p $(foreach d,ui/browsers scripting-engines,$(OUTPUT)util/$(d)/) 2> /dev/null
>
> > > > Is there actually a specific reason for the
> > > > $(shell sh -c '...')
> > > > around?
> > > > It looks superflous.
>
> > > I think the reason is is that nobody who touched that file really knew
> > > make all that well. Your version looks fine to me.
>
> > Ah, the reason is that they are not part of a rule but on the top-level
> > (and thus always executed).
>
> So it worked by luck! /me runs :-P

IMHO it's not really luck with GNU-make.

> More seriously, so there is a reason for that to be like that and you're
> not aware of any other shorter or more convenient way of achieving that

One (obvious) alternative is to have rules triggering on the
non-existence of these directories.

> goal, right?

Hmm, I'm not a "perf person". Which are the sufficient use-cases/tests
that one can do to play around with the Makefile?

`make -C tools/perf` is probably not enough.

Any hints anyone?

Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at

2010-08-16 15:50:51

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

Em Mon, Aug 16, 2010 at 05:43:47PM +0200, Bernd Petrovitsch escreveu:
> On Mon, 2010-08-16 at 12:29 -0300, Arnaldo Carvalho de Melo wrote:
> > More seriously, so there is a reason for that to be like that and you're
> > not aware of any other shorter or more convenient way of achieving that

> One (obvious) alternative is to have rules triggering on the
> non-existence of these directories.

Can you provide those please?

> > goal, right?

> Hmm, I'm not a "perf person". Which are the sufficient use-cases/tests
> that one can do to play around with the Makefile?
>
> `make -C tools/perf` is probably not enough.

Right, not enough, what those mkdir calls were added for was exactly for
a different usecase:

make -C tools/perf -O=~/build/perf/

So that it doesn't pollutes the source code directories with the object
files, behaving in a similar fashion as when using O= in the kernel
proper.

- Arnaldo

2010-08-17 11:59:01

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Mon, 2010-08-16 at 12:50 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 16, 2010 at 05:43:47PM +0200, Bernd Petrovitsch escreveu:
> > On Mon, 2010-08-16 at 12:29 -0300, Arnaldo Carvalho de Melo wrote:
> > > More seriously, so there is a reason for that to be like that and you're
> > > not aware of any other shorter or more convenient way of achieving that
>
> > One (obvious) alternative is to have rules triggering on the
> > non-existence of these directories.
>
> Can you provide those please?
[...]
> Right, not enough, what those mkdir calls were added for was exactly for
> a different usecase:
>
> make -C tools/perf -O=~/build/perf/

Thanks.

The following patch below at the end works for me. Alas, it is against
vanilla main line.
---- snip ----
Replace the global $(shell ...) lines quite at the top creating the output
directories with real rules.

Signed-of-by: Bernd Petrovitsch <[email protected]>
---
tools/perf/Makefile | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 230a0f7..c039fbc 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -157,10 +157,6 @@ all::
#
# Define NO_DWARF if you do not want debug-info analysis feature at all.

-$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
-$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
-$(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
-
$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
@$(SHELL_PATH) util/PERF-VERSION-GEN $(OUTPUT)
-include $(OUTPUT)PERF-VERSION-FILE
@@ -186,8 +182,6 @@ ifeq ($(ARCH),x86_64)
ARCH := x86
endif

-$(shell sh -c 'mkdir -p $(OUTPUT)arch/$(ARCH)/util/' 2> /dev/null)
-
# CFLAGS and LDFLAGS are for the users to override from the command line.

#
@@ -1012,6 +1006,13 @@ $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst perf-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
builtin-revert.o wt-status.o: wt-status.h

+# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
+# we depend the various files onto their directories.
+$(LIB_OBJS) $(BUILTIN_OBJS): $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)))
+# In the second step, we make a rule to actually create these directories
+$(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS))):
+ mkdir -p $@ 2>/dev/null
+
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
---- snip ----

Bernd
--
mobile: +43 664 4416156 http://www.sysprog.at/
Linux Software Development, Consulting and Services

2010-08-17 15:43:07

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

Em Tue, Aug 17, 2010 at 01:58:00PM +0200, Bernd Petrovitsch escreveu:
> On Mon, 2010-08-16 at 12:50 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Aug 16, 2010 at 05:43:47PM +0200, Bernd Petrovitsch escreveu:
> > > On Mon, 2010-08-16 at 12:29 -0300, Arnaldo Carvalho de Melo wrote:
> > > > More seriously, so there is a reason for that to be like that and you're
> > > > not aware of any other shorter or more convenient way of achieving that
> >
> > > One (obvious) alternative is to have rules triggering on the
> > > non-existence of these directories.
> >
> > Can you provide those please?
> [...]
> > Right, not enough, what those mkdir calls were added for was exactly for
> > a different usecase:
> >
> > make -C tools/perf -O=~/build/perf/
>
> Thanks.
>
> The following patch below at the end works for me. Alas, it is against
> vanilla main line.

Adding $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h there, to have it look:

+# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
+# we depend the various files onto their directories.
+$(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h: $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)))
+# In the second step, we make a rule to actually create these directories
+$(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS))):
+ mkdir -p $@ 2>/dev/null
+

As it was failing when I did:

rm -rf ~/build/perf
make -C tools/perf O=~/build/perf

With that it retains the existing functionality,

Thanks,

- Arnaldo

2010-08-17 16:10:20

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Die, 2010-08-17 at 12:42 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 17, 2010 at 01:58:00PM +0200, Bernd Petrovitsch escreveu:
[...]
> > The following patch below at the end works for me. Alas, it is against
> > vanilla main line.
>
> Adding $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h there, to have it look:
>
> +# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
> +# we depend the various files onto their directories.
> +$(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h: $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)))

Hmm, that adds that $(OUTPUT)PERF-VERSION-FILE and
$(OUTPUT)common-cmds.h (also) depends on the subdirectories of the other
objects (and thus it works always because at least one of them is a
subdirectory of $(OUTPUT)).
To be 110% anal, it should look like e.g.
+$(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h: $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)) $(OUTPUT))

or actually all targets.

> +# In the second step, we make a rule to actually create these directories
> +$(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS))):
> + mkdir -p $@ 2>/dev/null
> +

BTW there is no automatic variable or other make-construct to refer in
the dependencies on the own target. Therefore the copy-paste-the-
variables solution.

Perhaps an additional variable reduces clutter (and eases maintenance)?

+# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
+# we depend the various files onto their directories.
+DIRECTORY_DEPS = $(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
+$(DIRECTORY_DEPS): $(sort $(dir $(DIRECTORY_DEPS)))
+# In the second step, we make a rule to actually create these directories
+$(sort $(dir $(DIRECTORY_DEPS))):
+ mkdir -p $@ 2>/dev/null
+

That should guarantee that all directories from the targets are created.

> As it was failing when I did:
>
> rm -rf ~/build/perf
> make -C tools/perf O=~/build/perf
>
> With that it retains the existing functionality,

Ah, I `mkdir`ed the output directory explicitly before the `make` (and
after the `rm -rf`).


BTW which is the preferred tree to base patches on (for the "perf"
subsystem)?

Bernd
--
mobile: +43 664 4416156 http://www.sysprog.at/
Linux Software Development, Consulting and Services

2010-08-17 18:17:23

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

Em Tue, Aug 17, 2010 at 06:09:49PM +0200, Bernd Petrovitsch escreveu:
> On Die, 2010-08-17 at 12:42 -0300, Arnaldo Carvalho de Melo wrote:
> > +# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
> > +# we depend the various files onto their directories.
> > +$(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h: $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)))

> Hmm, that adds that $(OUTPUT)PERF-VERSION-FILE and
> $(OUTPUT)common-cmds.h (also) depends on the subdirectories of the other
> objects (and thus it works always because at least one of them is a
> subdirectory of $(OUTPUT)).
> To be 110% anal, it should look like e.g.
> +$(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h: $(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS)) $(OUTPUT))

> or actually all targets.

> > +# In the second step, we make a rule to actually create these directories
> > +$(sort $(dir $(LIB_OBJS) $(BUILTIN_OBJS))):
> > + mkdir -p $@ 2>/dev/null

> BTW there is no automatic variable or other make-construct to refer in
> the dependencies on the own target. Therefore the copy-paste-the-
> variables solution.

> Perhaps an additional variable reduces clutter (and eases maintenance)?

> +# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
> +# we depend the various files onto their directories.
> +DIRECTORY_DEPS = $(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
> +$(DIRECTORY_DEPS): $(sort $(dir $(DIRECTORY_DEPS)))
> +# In the second step, we make a rule to actually create these directories
> +$(sort $(dir $(DIRECTORY_DEPS))):
> + mkdir -p $@ 2>/dev/null

> That should guarantee that all directories from the targets are created.

> > As it was failing when I did:

> > rm -rf ~/build/perf
> > make -C tools/perf O=~/build/perf

> > With that it retains the existing functionality,

> Ah, I `mkdir`ed the output directory explicitly before the `make` (and
> after the `rm -rf`).

> BTW which is the preferred tree to base patches on (for the "perf"
> subsystem)?

git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip

perf/urgent for 2.6.36
perf/core for 2.6.37

I'm targeting perf/urgent for this fix.

Or git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6

Sometimes, when Ingo is busy and patches accumulate, perf/{core,urgent}
are where I put things to go to the branches with the same names on his
tree.

Please look if patch is OK with you.

- Arnaldo

commit 033a273f9836b592dd568abd0f655be469d66704
Author: Bernd Petrovitsch <[email protected]>
Date: Tue Aug 17 12:22:08 2010 -0300

perf tools: Fix build on POSIX shells

POSIX sh does not specify the brace expansion, so fix it by replacing the
global $(shell ...) lines quite at the top creating the output directories with
real rules.

Cc: Ingo Molnar <[email protected]>
Cc: Kusanagi Kouichi <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <1282046280.5822.4.camel@thorin>
Signed-off-by: Bernd Petrovitsch <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 41abb90..dcb9700 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -157,10 +157,6 @@ all::
#
# Define NO_DWARF if you do not want debug-info analysis feature at all.

-$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
-$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
-$(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
-
$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
@$(SHELL_PATH) util/PERF-VERSION-GEN $(OUTPUT)
-include $(OUTPUT)PERF-VERSION-FILE
@@ -186,8 +182,6 @@ ifeq ($(ARCH),x86_64)
ARCH := x86
endif

-$(shell sh -c 'mkdir -p $(OUTPUT)arch/$(ARCH)/util/' 2> /dev/null)
-
# CFLAGS and LDFLAGS are for the users to override from the command line.

#
@@ -268,6 +262,7 @@ export prefix bindir sharedir sysconfdir
CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar
RM = rm -f
+MKDIR = mkdir
TAR = tar
FIND = find
INSTALL = install
@@ -838,6 +833,7 @@ ifndef V
QUIET_CC = @echo ' ' CC $@;
QUIET_AR = @echo ' ' AR $@;
QUIET_LINK = @echo ' ' LINK $@;
+ QUIET_MKDIR = @echo ' ' MKDIR $@;
QUIET_BUILT_IN = @echo ' ' BUILTIN $@;
QUIET_GEN = @echo ' ' GEN $@;
QUIET_SUBDIR0 = +@subdir=
@@ -1012,6 +1008,14 @@ $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst perf-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
builtin-revert.o wt-status.o: wt-status.h

+# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
+# we depend the various files onto their directories.
+DIRECTORY_DEPS = $(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
+$(DIRECTORY_DEPS): $(sort $(dir $(DIRECTORY_DEPS)))
+# In the second step, we make a rule to actually create these directories
+$(sort $(dir $(DIRECTORY_DEPS))):
+ $(QUIET_MKDIR)$(MKDIR) -p $@ 2>/dev/null
+
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)

2010-08-18 08:19:41

by Bernd Petrovitsch

[permalink] [raw]
Subject: [tip:perf/urgent] perf tools: Fix build on POSIX shells

Commit-ID: 033a273f9836b592dd568abd0f655be469d66704
Gitweb: http://git.kernel.org/tip/033a273f9836b592dd568abd0f655be469d66704
Author: Bernd Petrovitsch <[email protected]>
AuthorDate: Tue, 17 Aug 2010 12:22:08 -0300
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 17 Aug 2010 12:22:08 -0300

perf tools: Fix build on POSIX shells

POSIX sh does not specify the brace expansion, so fix it by replacing the
global $(shell ...) lines quite at the top creating the output directories with
real rules.

Cc: Ingo Molnar <[email protected]>
Cc: Kusanagi Kouichi <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <1282046280.5822.4.camel@thorin>
Signed-off-by: Bernd Petrovitsch <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 41abb90..dcb9700 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -157,10 +157,6 @@ all::
#
# Define NO_DWARF if you do not want debug-info analysis feature at all.

-$(shell sh -c 'mkdir -p $(OUTPUT)scripts/{perl,python}/Perf-Trace-Util/' 2> /dev/null)
-$(shell sh -c 'mkdir -p $(OUTPUT)util/{ui/browsers,scripting-engines}/' 2> /dev/null)
-$(shell sh -c 'mkdir $(OUTPUT)bench' 2> /dev/null)
-
$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
@$(SHELL_PATH) util/PERF-VERSION-GEN $(OUTPUT)
-include $(OUTPUT)PERF-VERSION-FILE
@@ -186,8 +182,6 @@ ifeq ($(ARCH),x86_64)
ARCH := x86
endif

-$(shell sh -c 'mkdir -p $(OUTPUT)arch/$(ARCH)/util/' 2> /dev/null)
-
# CFLAGS and LDFLAGS are for the users to override from the command line.

#
@@ -268,6 +262,7 @@ export prefix bindir sharedir sysconfdir
CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar
RM = rm -f
+MKDIR = mkdir
TAR = tar
FIND = find
INSTALL = install
@@ -838,6 +833,7 @@ ifndef V
QUIET_CC = @echo ' ' CC $@;
QUIET_AR = @echo ' ' AR $@;
QUIET_LINK = @echo ' ' LINK $@;
+ QUIET_MKDIR = @echo ' ' MKDIR $@;
QUIET_BUILT_IN = @echo ' ' BUILTIN $@;
QUIET_GEN = @echo ' ' GEN $@;
QUIET_SUBDIR0 = +@subdir=
@@ -1012,6 +1008,14 @@ $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst perf-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
builtin-revert.o wt-status.o: wt-status.h

+# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
+# we depend the various files onto their directories.
+DIRECTORY_DEPS = $(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
+$(DIRECTORY_DEPS): $(sort $(dir $(DIRECTORY_DEPS)))
+# In the second step, we make a rule to actually create these directories
+$(sort $(dir $(DIRECTORY_DEPS))):
+ $(QUIET_MKDIR)$(MKDIR) -p $@ 2>/dev/null
+
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)

2010-08-18 09:47:40

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Don't use brace expansion.

On Die, 2010-08-17 at 15:16 -0300, Arnaldo Carvalho de Melo wrote:
[...]
> Please look if patch is OK with you.

FWIW now, I'm fully OK with it.

Bernd
--
mobile: +43 664 4416156 http://www.sysprog.at/
Linux Software Development, Consulting and Services