All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.