Patchwork [1/2,trivial] Generic elimination of auto-generated files

login
register
mail settings
Submitter Lluís Vilanova
Date April 13, 2012, 10:19 p.m.
Message ID <20120413221903.1079.85465.stgit@ginnungagap.bsc.es>
Download mbox | patch
Permalink /patch/152436/
State New
Headers show

Comments

Lluís Vilanova - April 13, 2012, 10:19 p.m.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 Makefile |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Stefan Hajnoczi - April 18, 2012, 1:06 p.m.
On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
> -	rm -f $(GENERATED_HEADERS)
> -	rm -f $(GENERATED_SOURCES)
> +	rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
> +	rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)

Why */$(f) */$(f)-timestamp?

Stefan
Lluís Vilanova - April 18, 2012, 1:37 p.m.
Stefan Hajnoczi writes:

> On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
>> -	rm -f $(GENERATED_HEADERS)
>> -	rm -f $(GENERATED_SOURCES)
>> +	rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>> +	rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)

> Why */$(f) */$(f)-timestamp?

Some of the files are generated in immediate subdirectories (e.g.,
libuser/trace.c-timestamp).

I could use the results of a call shell to "find" instead, so that it will
always find the right victims no matter where they are.


Lluis
Stefan Hajnoczi - April 18, 2012, 2:11 p.m.
On Wed, Apr 18, 2012 at 2:37 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
> Stefan Hajnoczi writes:
>
>> On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
>>> -    rm -f $(GENERATED_HEADERS)
>>> -    rm -f $(GENERATED_SOURCES)
>>> +    rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>> +    rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>
>> Why */$(f) */$(f)-timestamp?
>
> Some of the files are generated in immediate subdirectories (e.g.,
> libuser/trace.c-timestamp).
>
> I could use the results of a call shell to "find" instead, so that it will
> always find the right victims no matter where they are.

Ah, I get it.  This actually adds something that was missing previously.

I'm not a make expert, I think if we try we can find issues with any
approach to removing the files.  What you've done seems fine to me :).

Stefan
Andreas Färber - April 18, 2012, 3:12 p.m.
Am 18.04.2012 16:11, schrieb Stefan Hajnoczi:
> On Wed, Apr 18, 2012 at 2:37 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Stefan Hajnoczi writes:
>>
>>> On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
>>>> -    rm -f $(GENERATED_HEADERS)
>>>> -    rm -f $(GENERATED_SOURCES)
>>>> +    rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>>> +    rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>
>>> Why */$(f) */$(f)-timestamp?
>>
>> Some of the files are generated in immediate subdirectories (e.g.,
>> libuser/trace.c-timestamp).
>>
>> I could use the results of a call shell to "find" instead, so that it will
>> always find the right victims no matter where they are.
> 
> Ah, I get it.  This actually adds something that was missing previously.
> 
> I'm not a make expert, I think if we try we can find issues with any
> approach to removing the files.  What you've done seems fine to me :).

Er, nack. libuser has its own Makefile.user, so we should recurse into
libuser and rm from there. Is there an actual problem here apart from
*-timestamp?

Andreas
Lluís Vilanova - April 18, 2012, 3:32 p.m.
Stefan Hajnoczi writes:

> On Wed, Apr 18, 2012 at 2:37 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>> Stefan Hajnoczi writes:
>> 
>>> On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
>>>> -    rm -f $(GENERATED_HEADERS)
>>>> -    rm -f $(GENERATED_SOURCES)
>>>> +    rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>>> +    rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>> 
>>> Why */$(f) */$(f)-timestamp?
>> 
>> Some of the files are generated in immediate subdirectories (e.g.,
>> libuser/trace.c-timestamp).
>> 
>> I could use the results of a call shell to "find" instead, so that it will
>> always find the right victims no matter where they are.

> Ah, I get it.  This actually adds something that was missing previously.

> I'm not a make expert, I think if we try we can find issues with any
> approach to removing the files.  What you've done seems fine to me :).

What I meant is the following, so that it will always be correct no matter how
directories are organized in the future:

    rm -f $(foreach f,$(GENERATED_HEADERS),$(shell find $(BUILD_DIR) -name $(f) -o -name $(f)-timestamp))
    rm -f $(foreach f,$(GENERATED_SOURCES),$(shell find $(BUILD_DIR) -name $(f) -o -name $(f)-timestamp))

I'll resend this patch with the aforementioned changes.


Lluis
Lluís Vilanova - April 18, 2012, 3:58 p.m.
Andreas Färber writes:

> Am 18.04.2012 16:11, schrieb Stefan Hajnoczi:
>> On Wed, Apr 18, 2012 at 2:37 PM, Lluís Vilanova <vilanova@ac.upc.edu> wrote:
>>> Stefan Hajnoczi writes:
>>> 
>>>> On Sat, Apr 14, 2012 at 12:19:03AM +0200, Lluís Vilanova wrote:
>>>>> -    rm -f $(GENERATED_HEADERS)
>>>>> -    rm -f $(GENERATED_SOURCES)
>>>>> +    rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>>>> +    rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
>>> 
>>>> Why */$(f) */$(f)-timestamp?
>>> 
>>> Some of the files are generated in immediate subdirectories (e.g.,
>>> libuser/trace.c-timestamp).
>>> 
>>> I could use the results of a call shell to "find" instead, so that it will
>>> always find the right victims no matter where they are.
>> 
>> Ah, I get it.  This actually adds something that was missing previously.
>> 
>> I'm not a make expert, I think if we try we can find issues with any
>> approach to removing the files.  What you've done seems fine to me :).

> Er, nack. libuser has its own Makefile.user, so we should recurse into
> libuser and rm from there. Is there an actual problem here apart from
> *-timestamp?

Nope. So I suppose right now it's better to keep it as:

    rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
    rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)



Thanks,
  Lluis

Patch

diff --git a/Makefile b/Makefile
index 135cb72..8179966 100644
--- a/Makefile
+++ b/Makefile
@@ -23,7 +23,7 @@  ifeq ($(TRACE_BACKEND),dtrace)
 GENERATED_HEADERS += trace-dtrace.h
 endif
 GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
+GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
 
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
@@ -220,11 +220,11 @@  clean:
 	rm -f qom/*.o qom/*.d
 	rm -f qemu-img-cmds.h
 	rm -f trace/*.o trace/*.d
-	rm -f trace.c trace.h trace.c-timestamp trace.h-timestamp
 	rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
+	@# May not be present in GENERATED_HEADERS
 	rm -f trace-dtrace.h trace-dtrace.h-timestamp
-	rm -f $(GENERATED_HEADERS)
-	rm -f $(GENERATED_SOURCES)
+	rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
+	rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp */$(f) */$(f)-timestamp)
 	rm -rf $(qapi-dir)
 	$(MAKE) -C tests/tcg clean
 	for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \