[Qemu-devel] [PATCH] Makefile: Update unmodified config-devices.mak automatically

[Qemu-devel] [PATCH] Makefile: Update unmodified config-devices.mak automatically

[Stefan Weil]

This makes rebuilds after source updates easier
for most users (who don't edit config-devices.mak).

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 Makefile |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index c1fa08c..684365d 100644
--- a/Makefile
+++ b/Makefile
@@ -43,12 +43,22 @@ config-all-devices.mak: $(SUBDIR_DEVICES_MAK)
 
 %/config-devices.mak: default-configs/%.mak
 	$(call quiet-command,cat $< > $@.tmp, "  GEN   $@")
-	@if test -f $@ ; then \
-	  echo "WARNING: $@ out of date." ;\
-	  echo "Run \"make defconfig\" to regenerate." ; \
-	  rm $@.tmp ; \
+	@if test -f $@; then \
+	  if cmp -s $@.old $@ || cmp -s $@ $@.tmp; then \
+	    mv $@.tmp $@; \
+	    cp -p $@ $@.old; \
+	  else \
+	    if test -f $@.old; then \
+	      echo "WARNING: $@ (user modified) out of date.";\
+	    else \
+	      echo "WARNING: $@ out of date.";\
+	    fi; \
+	    echo "Run \"make defconfig\" to regenerate."; \
+	    rm $@.tmp; \
+	  fi; \
 	 else \
-	  mv $@.tmp $@ ; \
+	  mv $@.tmp $@; \
+	  cp -p $@ $@.old; \
 	 fi
 
 defconfig:
-- 
1.6.5

[Qemu-devel] Re: [PATCH] Makefile: Update unmodified config-devices.mak automatically

[Stefan Weil]

Michael S. Tsirkin schrieb:
> On Thu, Dec 24, 2009 at 04:03:17PM +0100, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>> On Thu, Dec 24, 2009 at 02:31:58PM +0100, Stefan Weil wrote:
>>>> Michael S. Tsirkin schrieb:
>>>>> On Sun, Dec 20, 2009 at 03:39:03PM +0100, Stefan Weil wrote:
>>>>>
>>>>>> This makes rebuilds after source updates easier
>>>>>> for most users (who don't edit config-devices.mak).
>>>>>>
>>>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>>>>
>>>>> Sorry about missing this and not commenting earlier.
>>>>>
>>>>> So the problem here is that it relies on keeping
>>>>> .old file around. This is a generated file, but
>>>>> - you don't remove it with make clean or distclean
>>>>>
>>>> As long as config-devices.mak is not removed,
>>>> config-devices.mak.old has to be kept, too.
>>> Yes but you keep it around even after config-devices.mak
>>> is removed.
>>>
>>>>> - it is not removed on error properly as it is not
>>>>> a target of makefile
>>>>> so it seems easy to get into a situation where
>>>>> a corrupted file will be created and the only way
>>>>> to get rid of it would be by manual rm command.
>>>>>
>>>> I don't think that this is a real world scenario.
>>>> The .old file is a simple copy of config-devices.mak.
>>> By the way, what if you really *do* want configuration that happens to
>>> exactly match the default at some point? There is no way to take that
>>> config and make it persistent, is there? With my approach, any edit of
>>> the file adding something will do.
>> You can't make your cake and have it.
>> Your include file fails exactly in this very scenary:
>>
>> #include default-config
>>
>> local modifications
>>
>> You change default-config, and you also have a different configuration.
>>
>> I don't see any advantage at using includes instead of copy the file,
>> and implement include directive just make things more complex IMHO.
>>
>>>>> Instead, what I think we should do is make
>>>>> the generated file *almost empty*
>>>>> and then it is easy to detect user tweaking
>>>>> it without keeping more state.
>>>>>
>>>> This is a matter of personal preferences.
>>>> Maybe other users who want to modify
>>>> config-devices.mak prefer to have a full
>>>> version where they can remove some lines
>>>> they don't need.
>>> IMO this encourages sloppiness, it is better to have a file which does
>>> 1. include default
>>> 2. change some values
>>> than have a full copy of said default and then try to figure out what
>>> did you change. If you want to look at the original, it is there.
>> This build system is lossely bassed on kernel config files. And in the
>> kernel, once that you have your .config, it will never be overwritten.
>>
>> Both options have its advantages and disadvantages.
>>
>> I am looking at this patch, and was thinking about reordering the
>> comparisons, but I am still not sure that my approach is better than
>> this one. Will think a bit more about it and include this one or
>> propose something similar in spirit.
>>
>> Later, Juan.
>
> IMO the really best approach is not to generate any files
> at all. This way we avoid confusion: if there is config
> file around, user created this file.
> But I don't care much, really.
>
> Whatever you do, please make sure that
> make defconfig itself does not print "run make defconfig",
> this is just silly.

Agreed, but this can be handled in a separate patch.
Only those who modify config-devices.mak manually
get this irritating message, so fixing it has lower
priority.

Antony, please send feedback if there is still something
why my patch cannot be applied as it is. It is much better
than the status quo, so I think it should be applied soon.

For a really satisfying solution, I'd prefer a build system
configured with "make menuconfig".

Kind regards,
Stefan Weil

Re: [Qemu-devel] [PATCH] Makefile: Update unmodified config-devices.mak automatically

[Anthony Liguori]

On 12/20/2009 08:39 AM, Stefan Weil wrote:
> This makes rebuilds after source updates easier
> for most users (who don't edit config-devices.mak).
>
> Signed-off-by: Stefan Weil<weil@mail.berlios.de>\
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   Makefile |   20 +++++++++++++++-----
>   1 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c1fa08c..684365d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -43,12 +43,22 @@ config-all-devices.mak: $(SUBDIR_DEVICES_MAK)
>
>   %/config-devices.mak: default-configs/%.mak
>   	$(call quiet-command,cat $<  >  $@.tmp, "  GEN   $@")
> -	@if test -f $@ ; then \
> -	  echo "WARNING: $@ out of date." ;\
> -	  echo "Run \"make defconfig\" to regenerate." ; \
> -	  rm $@.tmp ; \
> +	@if test -f $@; then \
> +	  if cmp -s $@.old $@ || cmp -s $@ $@.tmp; then \
> +	    mv $@.tmp $@; \
> +	    cp -p $@ $@.old; \
> +	  else \
> +	    if test -f $@.old; then \
> +	      echo "WARNING: $@ (user modified) out of date.";\
> +	    else \
> +	      echo "WARNING: $@ out of date.";\
> +	    fi; \
> +	    echo "Run \"make defconfig\" to regenerate."; \
> +	    rm $@.tmp; \
> +	  fi; \
>   	 else \
> -	  mv $@.tmp $@ ; \
> +	  mv $@.tmp $@; \
> +	  cp -p $@ $@.old; \
>   	 fi
>
>   defconfig:
>