public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: Don't use brace expansion.
@ 2010-08-16 12:41 Kusanagi Kouichi
  2010-08-16 13:30 ` Peter Zijlstra
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Kusanagi Kouichi @ 2010-08-16 12:41 UTC (permalink / raw)
  To: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo
  Cc: linux-kernel

DASH doesn't support brace expansion.

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
---
 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


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 12:41 [PATCH] perf tools: Don't use brace expansion Kusanagi Kouichi
@ 2010-08-16 13:30 ` Peter Zijlstra
  2010-08-16 14:09 ` Bernd Petrovitsch
  2010-08-16 15:24 ` [PATCH] perf tools: Don't use brace expansion Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 15+ messages in thread
From: Peter Zijlstra @ 2010-08-16 13:30 UTC (permalink / raw)
  To: Kusanagi Kouichi
  Cc: Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo,
	linux-kernel

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 <slash@ac.auone-net.jp>
> ---
>  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



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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 12:41 [PATCH] perf tools: Don't use brace expansion Kusanagi Kouichi
  2010-08-16 13:30 ` Peter Zijlstra
@ 2010-08-16 14:09 ` Bernd Petrovitsch
  2010-08-16 14:30   ` Peter Zijlstra
  2010-08-16 15:24 ` [PATCH] perf tools: Don't use brace expansion Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-16 14:09 UTC (permalink / raw)
  To: Kusanagi Kouichi
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo, linux-kernel

On Mon, 2010-08-16 at 21:41 +0900, Kusanagi Kouichi wrote:
> DASH doesn't support brace expansion.
> 
> Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> ---
>  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 : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 14:09 ` Bernd Petrovitsch
@ 2010-08-16 14:30   ` Peter Zijlstra
  2010-08-16 14:54     ` Bernd Petrovitsch
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2010-08-16 14:30 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo, linux-kernel

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.

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 14:30   ` Peter Zijlstra
@ 2010-08-16 14:54     ` Bernd Petrovitsch
  2010-08-16 15:29       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-16 14:54 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo, linux-kernel

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 : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 12:41 [PATCH] perf tools: Don't use brace expansion Kusanagi Kouichi
  2010-08-16 13:30 ` Peter Zijlstra
  2010-08-16 14:09 ` Bernd Petrovitsch
@ 2010-08-16 15:24 ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-16 15:24 UTC (permalink / raw)
  To: Kusanagi Kouichi
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

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 <slash@ac.auone-net.jp>
> ---
>  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

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 14:54     ` Bernd Petrovitsch
@ 2010-08-16 15:29       ` Arnaldo Carvalho de Melo
  2010-08-16 15:43         ` Bernd Petrovitsch
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-16 15:29 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 15:29       ` Arnaldo Carvalho de Melo
@ 2010-08-16 15:43         ` Bernd Petrovitsch
  2010-08-16 15:50           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-16 15:43 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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 : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 15:43         ` Bernd Petrovitsch
@ 2010-08-16 15:50           ` Arnaldo Carvalho de Melo
  2010-08-17 11:58             ` Bernd Petrovitsch
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-16 15:50 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-16 15:50           ` Arnaldo Carvalho de Melo
@ 2010-08-17 11:58             ` Bernd Petrovitsch
  2010-08-17 15:42               ` Arnaldo Carvalho de Melo
  2010-08-18  8:18               ` [tip:perf/urgent] perf tools: Fix build on POSIX shells tip-bot for Bernd Petrovitsch
  0 siblings, 2 replies; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-17 11:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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 <bernd@sysprog.at>
---
 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


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-17 11:58             ` Bernd Petrovitsch
@ 2010-08-17 15:42               ` Arnaldo Carvalho de Melo
  2010-08-17 16:09                 ` Bernd Petrovitsch
  2010-08-18  8:18               ` [tip:perf/urgent] perf tools: Fix build on POSIX shells tip-bot for Bernd Petrovitsch
  1 sibling, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-17 15:42 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-17 15:42               ` Arnaldo Carvalho de Melo
@ 2010-08-17 16:09                 ` Bernd Petrovitsch
  2010-08-17 18:16                   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-17 16:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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


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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-17 16:09                 ` Bernd Petrovitsch
@ 2010-08-17 18:16                   ` Arnaldo Carvalho de Melo
  2010-08-18  9:47                     ` Bernd Petrovitsch
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-08-17 18:16 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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 <bernd@sysprog.at>
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 <mingo@elte.hu>
    Cc: Kusanagi Kouichi <slash@ac.auone-net.jp>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Paul Mackerras <paulus@samba.org>
    LKML-Reference: <1282046280.5822.4.camel@thorin>
    Signed-off-by: Bernd Petrovitsch <bernd@sysprog.at>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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)
 

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

* [tip:perf/urgent] perf tools: Fix build on POSIX shells
  2010-08-17 11:58             ` Bernd Petrovitsch
  2010-08-17 15:42               ` Arnaldo Carvalho de Melo
@ 2010-08-18  8:18               ` tip-bot for Bernd Petrovitsch
  1 sibling, 0 replies; 15+ messages in thread
From: tip-bot for Bernd Petrovitsch @ 2010-08-18  8:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, slash, peterz, bernd,
	tglx, mingo

Commit-ID:  033a273f9836b592dd568abd0f655be469d66704
Gitweb:     http://git.kernel.org/tip/033a273f9836b592dd568abd0f655be469d66704
Author:     Bernd Petrovitsch <bernd@sysprog.at>
AuthorDate: Tue, 17 Aug 2010 12:22:08 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <mingo@elte.hu>
Cc: Kusanagi Kouichi <slash@ac.auone-net.jp>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1282046280.5822.4.camel@thorin>
Signed-off-by: Bernd Petrovitsch <bernd@sysprog.at>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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)
 

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

* Re: [PATCH] perf tools: Don't use brace expansion.
  2010-08-17 18:16                   ` Arnaldo Carvalho de Melo
@ 2010-08-18  9:47                     ` Bernd Petrovitsch
  0 siblings, 0 replies; 15+ messages in thread
From: Bernd Petrovitsch @ 2010-08-18  9:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Kusanagi Kouichi, Paul Mackerras, Ingo Molnar,
	linux-kernel

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


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

end of thread, other threads:[~2010-08-18  9:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-16 12:41 [PATCH] perf tools: Don't use brace expansion Kusanagi Kouichi
2010-08-16 13:30 ` Peter Zijlstra
2010-08-16 14:09 ` Bernd Petrovitsch
2010-08-16 14:30   ` Peter Zijlstra
2010-08-16 14:54     ` Bernd Petrovitsch
2010-08-16 15:29       ` Arnaldo Carvalho de Melo
2010-08-16 15:43         ` Bernd Petrovitsch
2010-08-16 15:50           ` Arnaldo Carvalho de Melo
2010-08-17 11:58             ` Bernd Petrovitsch
2010-08-17 15:42               ` Arnaldo Carvalho de Melo
2010-08-17 16:09                 ` Bernd Petrovitsch
2010-08-17 18:16                   ` Arnaldo Carvalho de Melo
2010-08-18  9:47                     ` Bernd Petrovitsch
2010-08-18  8:18               ` [tip:perf/urgent] perf tools: Fix build on POSIX shells tip-bot for Bernd Petrovitsch
2010-08-16 15:24 ` [PATCH] perf tools: Don't use brace expansion Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox