Patchwork make trace options use autoconfy names

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 14, 2010, 11:50 a.m.
Message ID <1289735432-8776-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/71102/
State New
Headers show

Comments

Paolo Bonzini - Nov. 14, 2010, 11:50 a.m.
These are not in any release, so I am splitting them off the other
patch for autoconfy command line and not introducing deprecation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Andreas Färber - Nov. 14, 2010, 1:38 p.m.
Am 14.11.2010 um 12:50 schrieb Paolo Bonzini:

> These are not in any release, so I am splitting them off the other
> patch for autoconfy command line and not introducing deprecation.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> configure |    8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 50fb3cd..4e79059 100755
> --- a/configure
> +++ b/configure
> @@ -523,9 +523,9 @@ for opt do
>   ;;
>   --target-list=*) target_list="$optarg"
>   ;;
> -  --trace-backend=*) trace_backend="$optarg"
> +  --enable-trace-backend=*) trace_backend="$optarg"
>   ;;

This one's okay...

> -  --trace-file=*) trace_file="$optarg"
> +  --enable-trace-file=*) trace_file="$optarg"
>   ;;

but this should be --with-trace-file=... please. It is not being  
enabled, just set to a different value.

Andreas
Paolo Bonzini - Nov. 14, 2010, 1:52 p.m.
On 11/14/2010 02:38 PM, Andreas Färber wrote:
>> - --trace-file=*) trace_file="$optarg"
>> + --enable-trace-file=*) trace_file="$optarg"
>> ;;
>
> but this should be --with-trace-file=... please. It is not being
> enabled, just set to a different value.

--with-* should be reserved for library paths, but I can change it if 
people prefer it that way.

Paolo
Stefan Hajnoczi - Nov. 15, 2010, 2:17 p.m.
On Sun, Nov 14, 2010 at 1:52 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 11/14/2010 02:38 PM, Andreas Färber wrote:
>>>
>>> - --trace-file=*) trace_file="$optarg"
>>> + --enable-trace-file=*) trace_file="$optarg"
>>> ;;
>>
>> but this should be --with-trace-file=... please. It is not being
>> enabled, just set to a different value.
>
> --with-* should be reserved for library paths, but I can change it if people
> prefer it that way.

Actually I think we have something similar to overriding --prefix here
>:).  It's a path that you can set at ./configure time.

So is it not okay to use --trace-file=<filename>?  But I know nothing
of autoconf and --enable-* or --with-* sort of make sense too.

Stefan
Paolo Bonzini - Nov. 15, 2010, 3:48 p.m.
On 11/15/2010 03:17 PM, Stefan Hajnoczi wrote:
> On Sun, Nov 14, 2010 at 1:52 PM, Paolo Bonzini<pbonzini@redhat.com>  wrote:
>> On 11/14/2010 02:38 PM, Andreas Färber wrote:
>>>>
>>>> - --trace-file=*) trace_file="$optarg"
>>>> + --enable-trace-file=*) trace_file="$optarg"
>>>> ;;
>>>
>>> but this should be --with-trace-file=... please. It is not being
>>> enabled, just set to a different value.
>>
>> --with-* should be reserved for library paths, but I can change it if people
>> prefer it that way.
>
> Actually I think we have something similar to overriding --prefix here
> :).  It's a path that you can set at ./configure time.

Yeah, that's true.  However...

> So is it not okay to use --trace-file=<filename>?

... Autoconf would not allow unknown options not starting with --enable- 
or --with-.

The rationale to avoid incompatible options in QEMU is this: suppose you 
have a project using Autoconf (e.g. GCC) and you want to drop QEMU as a 
subdirectory in there, e.g. to run the GCC testsuite under QEMU usermode 
emulation (GCC can already do this for other simulators).  To pass 
options to QEMU's configure, you can include them in GCC's commandline. 
  The script will simply pass the option down to QEMU and it will be 
processed there.  However, if you pass --trace-file to GCC's configure 
script, it will complain and stop.

Probably I would use something like --enable-trace-backend=simple:trace- 
if I was adding something similar to an autoconfiscated project.  But 
unless it provides some additional benefit (as is the case with 
cross-compilation support) I want to keep the syntactic changes in my 
patches to the minimum.

> But I know nothing of autoconf and --enable-* or --with-* sort of
> make sense too.

Whatever, I have to repost the other series anyway, so I'll change to 
--with-.

Paolo
Andreas Färber - Nov. 15, 2010, 7:50 p.m.
Am 15.11.2010 um 16:48 schrieb Paolo Bonzini:

> On 11/15/2010 03:17 PM, Stefan Hajnoczi wrote:
>> On Sun, Nov 14, 2010 at 1:52 PM, Paolo  
>> Bonzini<pbonzini@redhat.com>  wrote:
>>> On 11/14/2010 02:38 PM, Andreas Färber wrote:
>>>>>
>>>>> - --trace-file=*) trace_file="$optarg"
>>>>> + --enable-trace-file=*) trace_file="$optarg"
>>>>> ;;
>>>>
>>>> but this should be --with-trace-file=... please. It is not being
>>>> enabled, just set to a different value.
>>>
>>> --with-* should be reserved for library paths, but I can change it  
>>> if people
>>> prefer it that way.

I did think of the argument as a file name, sort of a relative path...

>> Actually I think we have something similar to overriding --prefix  
>> here
>> :).  It's a path that you can set at ./configure time.
>
> Yeah, that's true.  However...
>
>> So is it not okay to use --trace-file=<filename>?
>
> ... Autoconf would not allow unknown options not starting with -- 
> enable- or --with-.
>
> The rationale to avoid incompatible options in QEMU is this: suppose  
> you have a project using Autoconf (e.g. GCC) and you want to drop  
> QEMU as a subdirectory in there, e.g. to run the GCC testsuite under  
> QEMU usermode emulation (GCC can already do this for other  
> simulators).  To pass options to QEMU's configure, you can include  
> them in GCC's commandline.  The script will simply pass the option  
> down to QEMU and it will be processed there.  However, if you pass -- 
> trace-file to GCC's configure script, it will complain and stop.
>
> Probably I would use something like --enable-trace- 
> backend=simple:trace- if I was adding something similar to an  
> autoconfiscated project.  But unless it provides some additional  
> benefit (as is the case with cross-compilation support) I want to  
> keep the syntactic changes in my patches to the minimum.
>
>> But I know nothing of autoconf and --enable-* or --with-* sort of
>> make sense too.
>
> Whatever, I have to repost the other series anyway, so I'll change  
> to --with-.

Thinking more about it, what about --enable-simple-trace=...,  
callapsing the two options into one?

Another autoconf way to pass this stuff would we ./configure ...  
TRACE_FILE=...

I wouldn't mind either way though, just noticed that the --enable- 
trace-file suggestion by autoconf convention would allow --disable- 
trace-file. Similar issue for --without-trace-file though.

Andreas

Patch

diff --git a/configure b/configure
index 50fb3cd..4e79059 100755
--- a/configure
+++ b/configure
@@ -523,9 +523,9 @@  for opt do
   ;;
   --target-list=*) target_list="$optarg"
   ;;
-  --trace-backend=*) trace_backend="$optarg"
+  --enable-trace-backend=*) trace_backend="$optarg"
   ;;
-  --trace-file=*) trace_file="$optarg"
+  --enable-trace-file=*) trace_file="$optarg"
   ;;
   --enable-gprof) gprof="yes"
   ;;
@@ -906,8 +906,8 @@  echo "  --enable-docs            enable documentation build"
 echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
-echo "  --trace-backend=B        Trace backend nop simple ust"
-echo "  --trace-file=NAME        Full PATH,NAME of file to store traces"
+echo "  --enable-trace-backend=B Trace backend nop simple ust"
+echo "  --enable-trace-file=NAME Full PATH,NAME of file to store traces"
 echo "                           Default:trace-<pid>"
 echo "  --disable-spice          disable spice"
 echo "  --enable-spice           enable spice"