[Qemu-devel] [PATCH] configure: re-add current options in config-host.mak

[Qemu-devel] [PATCH] configure: re-add current options in config-host.mak

[Michael S. Tsirkin]

commit bdf523e6923f1e667055e947e71fc600549cefae
    "configure: Add config.status to recreate the current configuration"
moved current configure flags to config.status

This makes sense, but this makes it harder to bisect problems that
trigger with a specific set of flags only, since old configure is not
smart enough to find them there.

Simply add the options as a comment in the format
old configure expected - they are then ignored by the new configure.

Cc: Stefan Weil <sw@weilnetz.de>
Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 508f6a5..f6d5ccd 100755
--- a/configure
+++ b/configure
@@ -3797,6 +3797,7 @@ config_host_mak="config-host.mak"
 echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
 
 echo "# Automatically generated by configure - do not modify" > $config_host_mak
+sed -n -e 's/exec/# Configured with:/p' config.status >> $config_host_mak
 echo >> $config_host_mak
 
 echo all: >> $config_host_mak
-- 
MST

Re: [Qemu-devel] [PATCH] configure: re-add current options in config-host.mak

[Stefan Weil]

Am 24.11.2013 10:37, schrieb Michael S. Tsirkin:
> commit bdf523e6923f1e667055e947e71fc600549cefae
>     "configure: Add config.status to recreate the current configuration"
> moved current configure flags to config.status
>
> This makes sense, but this makes it harder to bisect problems that
> trigger with a specific set of flags only, since old configure is not
> smart enough to find them there.
>
> Simply add the options as a comment in the format
> old configure expected - they are then ignored by the new configure.
>
> Cc: Stefan Weil <sw@weilnetz.de>
> Cc: Fam Zheng <famz@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  configure | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 508f6a5..f6d5ccd 100755
> --- a/configure
> +++ b/configure
> @@ -3797,6 +3797,7 @@ config_host_mak="config-host.mak"
>  echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
>  
>  echo "# Automatically generated by configure - do not modify" > $config_host_mak
> +sed -n -e 's/exec/# Configured with:/p' config.status >> $config_host_mak
>  echo >> $config_host_mak
>  
>  echo all: >> $config_host_mak

Do we really need this? It only makes git bisect iterations from
versions after 2013-11-19 to versions before that date easier. If a
typical git bisect needs 10 steps, then in the worst cast 5 of them
won't automatically run configure. This is normally not a big problem
because there is already a configuration, and many changes of file
configure don't change that configuration.

It's always possible to run configure explicitly at each step of the
bisection process, so in case of doubt there is an easy fall-back
solution. Before I introduced automatic reconfiguration, calling
configure + make was normal for git bisect.

I'd prefer to avoid code like this patch which is only marginally useful
in a very specific development use case. People who can run git bisect
will be able to help themselves if they really get a problem without the
patch.

Regards,
Stefan

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Paolo Bonzini]

Il 24/11/2013 12:04, Stefan Weil ha scritto:
> Do we really need this? It only makes git bisect iterations from
> versions after 2013-11-19 to versions before that date easier. If a
> typical git bisect needs 10 steps, then in the worst cast 5 of them
> won't automatically run configure. This is normally not a big problem
> because there is already a configuration, and many changes of file
> configure don't change that configuration.
> 
> It's always possible to run configure explicitly at each step of the
> bisection process, so in case of doubt there is an easy fall-back
> solution. Before I introduced automatic reconfiguration, calling
> configure + make was normal for git bisect.
> 
> I'd prefer to avoid code like this patch which is only marginally useful
> in a very specific development use case. People who can run git bisect
> will be able to help themselves if they really get a problem without the
> patch.

I think anything that makes it easier for users to do bisections instead
of us, and anything that makes it easier to script bisections ("git
bisect run"), is valuable.

Thanks Michael, patch

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

for 1.7 too.

Paolo

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Peter Maydell]

On 25 November 2013 09:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I think anything that makes it easier for users to do bisections instead
> of us, and anything that makes it easier to script bisections ("git
> bisect run"), is valuable.

Personally I've taken to doing bisections with a complete
"blow away build tree and start from scratch" process because
it's more reliable...

-- PMM

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Michael S. Tsirkin]

On Mon, Nov 25, 2013 at 10:53:44AM +0100, Paolo Bonzini wrote:
> Il 24/11/2013 12:04, Stefan Weil ha scritto:
> > Do we really need this? It only makes git bisect iterations from
> > versions after 2013-11-19 to versions before that date easier. If a
> > typical git bisect needs 10 steps, then in the worst cast 5 of them
> > won't automatically run configure. This is normally not a big problem
> > because there is already a configuration, and many changes of file
> > configure don't change that configuration.
> > 
> > It's always possible to run configure explicitly at each step of the
> > bisection process, so in case of doubt there is an easy fall-back
> > solution. Before I introduced automatic reconfiguration, calling
> > configure + make was normal for git bisect.
> > 
> > I'd prefer to avoid code like this patch which is only marginally useful
> > in a very specific development use case. People who can run git bisect
> > will be able to help themselves if they really get a problem without the
> > patch.
> 
> I think anything that makes it easier for users to do bisections instead
> of us, and anything that makes it easier to script bisections ("git
> bisect run"), is valuable.
> 
> Thanks Michael, patch
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> for 1.7 too.
> 
> Paolo

IMHO it's helpful but it's too late for controversial patches now.
So I won't push this to Anthony unless Stefan reconsiders and acks.

-- 
MST

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 November 2013 09:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I think anything that makes it easier for users to do bisections instead
>> of us, and anything that makes it easier to script bisections ("git
>> bisect run"), is valuable.
>
> Personally I've taken to doing bisections with a complete
> "blow away build tree and start from scratch" process because
> it's more reliable...

ccache speeds that up quite nicely :)

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Stefan Weil]

Am 25.11.2013 12:37, schrieb Michael S. Tsirkin:
> On Mon, Nov 25, 2013 at 10:53:44AM +0100, Paolo Bonzini wrote:
>> Il 24/11/2013 12:04, Stefan Weil ha scritto:
>>> Do we really need this? It only makes git bisect iterations from
>>> versions after 2013-11-19 to versions before that date easier. If a
>>> typical git bisect needs 10 steps, then in the worst cast 5 of them
>>> won't automatically run configure. This is normally not a big problem
>>> because there is already a configuration, and many changes of file
>>> configure don't change that configuration.
>>>
>>> It's always possible to run configure explicitly at each step of the
>>> bisection process, so in case of doubt there is an easy fall-back
>>> solution. Before I introduced automatic reconfiguration, calling
>>> configure + make was normal for git bisect.
>>>
>>> I'd prefer to avoid code like this patch which is only marginally useful
>>> in a very specific development use case. People who can run git bisect
>>> will be able to help themselves if they really get a problem without the
>>> patch.
>> I think anything that makes it easier for users to do bisections instead
>> of us, and anything that makes it easier to script bisections ("git
>> bisect run"), is valuable.
>>
>> Thanks Michael, patch
>>
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> for 1.7 too.
>>
>> Paolo
> IMHO it's helpful but it's too late for controversial patches now.
> So I won't push this to Anthony unless Stefan reconsiders and acks.

If it helps you and Paolo (and maybe others), I don't mind if you push
the patch. Technically it looks fine, therefore

Acked-by: Stefan Weil <sw@weilnetz.de>

Let me just add some information from my personal experience with git
bisect.

I don't run "git bisect" on QEMU very often - maybe once per month. "git
bisect run" was never a feasible option because in most cases I had to
inspect the output on QEMU's framebuffer. Nearly all bisections needed
manual intervention, either because the configure options changed
(removal of audio options), code was broken or for other reasons. On
each step I try an incremental build first, if that fails, a clean build
follows (I build always out of tree, so it's easy to really clean
everything).

Cheers,
Stefan

Re: [Qemu-devel] [PATCH 1.7] configure: re-add current options in config-host.mak

[Fam Zheng]

On 2013年11月25日 20:04, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 25 November 2013 09:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> I think anything that makes it easier for users to do bisections instead
>>> of us, and anything that makes it easier to script bisections ("git
>>> bisect run"), is valuable.
>>
>> Personally I've taken to doing bisections with a complete
>> "blow away build tree and start from scratch" process because
>> it's more reliable...
>
> ccache speeds that up quite nicely :)
>

This is fantastic! Thanks for the tip.

Fam