[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Anthony Liguori]

malc wrote:
> On Thu, 10 Sep 2009, Juan Quintela wrote:
>
>   
>> malc <av1474@comtv.ru> wrote:
>>     
>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
>>>
>>>       
>>>> From: Juan Quintela <quintela@redhat.com>
>>>>
>>>> With cc-option we are testing if gcc just accept a particular option, we 
>>>> don't need CFLAGS at all.  And this fixes the recursive problem with 
>>>> CFLAGS
>>>>         
>>> This is nonsense, previous options, those in CFLAGS, might conflict with
>>> the new ones.
>>>       
>> The only thing that we are testing is if gcc support that _option_
>>
>> What is the use case tat you have in mind?  A first grep on gcc man page
>> don't show options that conflict with each other.
>>     
>
> If you want artificial exmaples i can come up with plenty, and from the
> top of my head -m486 with -msse2 are quite incompatible with each other,
> furthermore, point is this - testing one option in isolation is broken.
>   

I think this is a pretty valid point.  I think it's worth while to try 
and better understand what the recursion problem was in the first place.

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Anthony Liguori]

Juan Quintela wrote:
> malc <av1474@comtv.ru> wrote:
>   
>> On Thu, 10 Sep 2009, Juan Quintela wrote:
>>
>>     
>>> malc <av1474@comtv.ru> wrote:
>>>       
>>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
>>>>
>>>>         
>>>>> From: Juan Quintela <quintela@redhat.com>
>>>>>
>>>>> With cc-option we are testing if gcc just accept a particular option, we 
>>>>> don't need CFLAGS at all.  And this fixes the recursive problem with 
>>>>> CFLAGS
>>>>>           
>>>> This is nonsense, previous options, those in CFLAGS, might conflict with
>>>> the new ones.
>>>>         
>>> The only thing that we are testing is if gcc support that _option_
>>>
>>> What is the use case tat you have in mind?  A first grep on gcc man page
>>> don't show options that conflict with each other.
>>>       
>> If you want artificial exmaples i can come up with plenty, and from the
>> top of my head -m486 with -msse2 are quite incompatible with each other,
>> furthermore, point is this - testing one option in isolation is broken.
>>     
>
> Ok. For the case that we were using, it don't matter at all.  But in
> general, there "could" (it is only one cc-option call in all sources).
>
> Anthony, what do you preffer:
> - revert the patch and add another one that changes += by :=
>   

No, I don't want to revert this patch and switch to :=.  += should 
work.  I don't understand why it doesn't.

What I'd prefer is for someone to figure out the root cause of += not 
working for us.  If we can't, I'd like a big fat comment stating that 
it's a known deficiency and we'll move on.

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Anthony Liguori wrote:
> Juan Quintela wrote:
> > malc <av1474@comtv.ru> wrote:
> >> On Thu, 10 Sep 2009, Juan Quintela wrote:
> >>> malc <av1474@comtv.ru> wrote:
> >>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
> >>>>> From: Juan Quintela <quintela@redhat.com>
> >>>>>
> >>>>> With cc-option we are testing if gcc just accept a particular option,
> >>>>> we don't need CFLAGS at all.  And this fixes the recursive problem
> >>>>> with CFLAGS
> >>>>
> >>>> This is nonsense, previous options, those in CFLAGS, might conflict
> >>>> with the new ones.
> >>>
> >>> The only thing that we are testing is if gcc support that _option_
> >>>
> >>> What is the use case tat you have in mind?  A first grep on gcc man
> >>> page don't show options that conflict with each other.
> >>
> >> If you want artificial exmaples i can come up with plenty, and from the
> >> top of my head -m486 with -msse2 are quite incompatible with each other,
> >> furthermore, point is this - testing one option in isolation is broken.
> >
> > Ok. For the case that we were using, it don't matter at all.  But in
> > general, there "could" (it is only one cc-option call in all sources).
> >
> > Anthony, what do you preffer:
> > - revert the patch and add another one that changes += by :=
>
> No, I don't want to revert this patch and switch to :=.  += should
> work.  I don't understand why it doesn't.
>
> What I'd prefer is for someone to figure out the root cause of += not
> working for us.  If we can't, I'd like a big fat comment stating that
> it's a known deficiency and we'll move on.

"make" complains about an infinite recursive assignment because the first 
assignment of CFLAGS defines a recursively-expanded variable and the right 
value is a macro which uses the CFLAGS.

As explained in 
http://www.gnu.org/software/make/manual/html_node/Appending.html , it can be 
fixed by making CFLAGS a simply-expanded variable at its first assignment.

I have submitted a patch.
-- 
Thomas

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Juan Quintela]

Thomas Monjalon <thomas@monjalon.net> wrote:
> Anthony Liguori wrote:
>> Juan Quintela wrote:
>> > malc <av1474@comtv.ru> wrote:
>> >> On Thu, 10 Sep 2009, Juan Quintela wrote:
>> >>> malc <av1474@comtv.ru> wrote:
>> >>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
>> >>>>> From: Juan Quintela <quintela@redhat.com>
>> >>>>>
>> >>>>> With cc-option we are testing if gcc just accept a particular option,
>> >>>>> we don't need CFLAGS at all.  And this fixes the recursive problem
>> >>>>> with CFLAGS
>> >>>>
>> >>>> This is nonsense, previous options, those in CFLAGS, might conflict
>> >>>> with the new ones.
>> >>>
>> >>> The only thing that we are testing is if gcc support that _option_
>> >>>
>> >>> What is the use case tat you have in mind?  A first grep on gcc man
>> >>> page don't show options that conflict with each other.
>> >>
>> >> If you want artificial exmaples i can come up with plenty, and from the
>> >> top of my head -m486 with -msse2 are quite incompatible with each other,
>> >> furthermore, point is this - testing one option in isolation is broken.
>> >
>> > Ok. For the case that we were using, it don't matter at all.  But in
>> > general, there "could" (it is only one cc-option call in all sources).
>> >
>> > Anthony, what do you preffer:
>> > - revert the patch and add another one that changes += by :=
>>
>> No, I don't want to revert this patch and switch to :=.  += should
>> work.  I don't understand why it doesn't.
>>
>> What I'd prefer is for someone to figure out the root cause of += not
>> working for us.  If we can't, I'd like a big fat comment stating that
>> it's a known deficiency and we'll move on.
>
> "make" complains about an infinite recursive assignment because the first 
> assignment of CFLAGS defines a recursively-expanded variable and the right 
> value is a macro which uses the CFLAGS.
>
> As explained in 
> http://www.gnu.org/software/make/manual/html_node/Appending.html , it can be 
> fixed by making CFLAGS a simply-expanded variable at its first assignment.
>
> I have submitted a patch.

Thanks for the explanation.
I haven't yet seen your patch, but I agree that right thing to do is
do the 1st assignment as QEMU_CFLAGS := foo

We can't really use CFLAGS, because CFLAGS need to work from the command
line

make CFLAGS="-O0 -g"

or I am lossing something obvious?

Later, Juan.

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Juan Quintela wrote:
> Thomas Monjalon <thomas@monjalon.net> wrote:
> > Anthony Liguori wrote:
> >> What I'd prefer is for someone to figure out the root cause of += not
> >> working for us.  If we can't, I'd like a big fat comment stating that
> >> it's a known deficiency and we'll move on.
> >
> > "make" complains about an infinite recursive assignment because the first
> > assignment of CFLAGS defines a recursively-expanded variable and the
> > right value is a macro which uses the CFLAGS.
> >
> > As explained in
> > http://www.gnu.org/software/make/manual/html_node/Appending.html , it can
> > be fixed by making CFLAGS a simply-expanded variable at its first
> > assignment.
> >
> > I have submitted a patch.
>
> Thanks for the explanation.
> I haven't yet seen your patch, but I agree that right thing to do is
> do the 1st assignment as QEMU_CFLAGS := foo
>
> We can't really use CFLAGS, because CFLAGS need to work from the command
> line
>
> make CFLAGS="-O0 -g"
>
> or I am lossing something obvious?

In order to use the variable from the command line, we should use the 
syntax "override" before each CFLAGS modification. This way, it is possible 
to append options after the user ones.
If override is not used, "make" expand the variable by using the command line 
value in priority, which overwrites the Makefile value.

I could send another patch for that but it seems that my mails are filtered 
out by the mailing list.
-- 
Thomas

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Juan Quintela]

Thomas Monjalon <thomas@monjalon.net> wrote:
> Juan Quintela wrote:
>> Thomas Monjalon <thomas@monjalon.net> wrote:
>> > Anthony Liguori wrote:
>> >> What I'd prefer is for someone to figure out the root cause of += not
>> >> working for us.  If we can't, I'd like a big fat comment stating that
>> >> it's a known deficiency and we'll move on.
>> >
>> > "make" complains about an infinite recursive assignment because the first
>> > assignment of CFLAGS defines a recursively-expanded variable and the
>> > right value is a macro which uses the CFLAGS.
>> >
>> > As explained in
>> > http://www.gnu.org/software/make/manual/html_node/Appending.html , it can
>> > be fixed by making CFLAGS a simply-expanded variable at its first
>> > assignment.
>> >
>> > I have submitted a patch.
>>
>> Thanks for the explanation.
>> I haven't yet seen your patch, but I agree that right thing to do is
>> do the 1st assignment as QEMU_CFLAGS := foo
>>
>> We can't really use CFLAGS, because CFLAGS need to work from the command
>> line
>>
>> make CFLAGS="-O0 -g"
>>
>> or I am lossing something obvious?
>
> In order to use the variable from the command line, we should use the 
> syntax "override" before each CFLAGS modification. This way, it is possible 
> to append options after the user ones.
> If override is not used, "make" expand the variable by using the command line 
> value in priority, which overwrites the Makefile value.
>
qemu only uses CFLAGS twice in the build system (everything interesting
goes in QEMU_CFLAGS)

(this in configure)
CFLAGS="-g $CFLAGS"
if test "$debug" = "no" ; then
  CFLAGS="-O2 $CFLAGS"
fi

echo "CFLAGS=$CFLAGS" >> $config_host_mak

and 
%.o: %.c
	$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")

(optionrom still uses them)

I think that no override is needed (we can use := for QEMU_CFLAGS)

> I could send another patch for that but it seems that my mails are filtered 
> out by the mailing list.

Can you sent it to me then, I will repost

Later, Juan.

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Anthony Liguori wrote:
> Juan Quintela wrote:
> > malc <av1474@comtv.ru> wrote:
> >> On Thu, 10 Sep 2009, Juan Quintela wrote:
> >>> malc <av1474@comtv.ru> wrote:
> >>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
> >>>>> From: Juan Quintela <quintela@redhat.com>
> >>>>>
> >>>>> With cc-option we are testing if gcc just accept a particular option,
> >>>>> we don't need CFLAGS at all.  And this fixes the recursive problem
> >>>>> with CFLAGS
> >>>>
> >>>> This is nonsense, previous options, those in CFLAGS, might conflict
> >>>> with the new ones.
> >>>
> >>> The only thing that we are testing is if gcc support that _option_
> >>>
> >>> What is the use case tat you have in mind?  A first grep on gcc man
> >>> page don't show options that conflict with each other.
> >>
> >> If you want artificial exmaples i can come up with plenty, and from the
> >> top of my head -m486 with -msse2 are quite incompatible with each other,
> >> furthermore, point is this - testing one option in isolation is broken.
> >
> > Ok. For the case that we were using, it don't matter at all.  But in
> > general, there "could" (it is only one cc-option call in all sources).
> >
> > Anthony, what do you preffer:
> > - revert the patch and add another one that changes += by :=
>
> No, I don't want to revert this patch and switch to :=.  += should
> work.  I don't understand why it doesn't.
>
> What I'd prefer is for someone to figure out the root cause of += not
> working for us.  If we can't, I'd like a big fat comment stating that
> it's a known deficiency and we'll move on.

"make" complains about an infinite recursive assignment because the first 
assignment of CFLAGS defines a recursively-expanded variable and the right 
value is a macro which uses the CFLAGS.

As explained in 
http://www.gnu.org/software/make/manual/html_node/Appending.html , it can be 
fixed by making CFLAGS a simply-expanded variable at its first assignment.

I have submitted a patch.
-- 
Thomas

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Juan Quintela wrote:
> Thomas Monjalon <thomas@monjalon.net> wrote:
> > Anthony Liguori wrote:
> >> What I'd prefer is for someone to figure out the root cause of += not
> >> working for us.  If we can't, I'd like a big fat comment stating that
> >> it's a known deficiency and we'll move on.
> >
> > "make" complains about an infinite recursive assignment because the first
> > assignment of CFLAGS defines a recursively-expanded variable and the
> > right value is a macro which uses the CFLAGS.
> >
> > As explained in
> > http://www.gnu.org/software/make/manual/html_node/Appending.html , it can
> > be fixed by making CFLAGS a simply-expanded variable at its first
> > assignment.
> >
> > I have submitted a patch.
>
> Thanks for the explanation.
> I haven't yet seen your patch, but I agree that right thing to do is
> do the 1st assignment as QEMU_CFLAGS := foo
>
> We can't really use CFLAGS, because CFLAGS need to work from the command
> line
>
> make CFLAGS="-O0 -g"
>
> or I am lossing something obvious?

In order to use the variable from the command line, we should use the 
syntax "override" before each CFLAGS modification. This way, it is possible 
to append options after the user ones.
If override is not used, "make" expand the variable by using the command line 
value in priority, which overwrites the Makefile value.
-- 
Thomas

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Anthony Liguori wrote:
> Juan Quintela wrote:
> > malc <av1474@comtv.ru> wrote:
> >> On Thu, 10 Sep 2009, Juan Quintela wrote:
> >>> malc <av1474@comtv.ru> wrote:
> >>>> On Wed, 9 Sep 2009, Anthony Liguori wrote:
> >>>>> From: Juan Quintela <quintela@redhat.com>
> >>>>>
> >>>>> With cc-option we are testing if gcc just accept a particular option,
> >>>>> we don't need CFLAGS at all.  And this fixes the recursive problem
> >>>>> with CFLAGS
> >>>>
> >>>> This is nonsense, previous options, those in CFLAGS, might conflict
> >>>> with the new ones.
> >>>
> >>> The only thing that we are testing is if gcc support that _option_
> >>>
> >>> What is the use case tat you have in mind?  A first grep on gcc man
> >>> page don't show options that conflict with each other.
> >>
> >> If you want artificial exmaples i can come up with plenty, and from the
> >> top of my head -m486 with -msse2 are quite incompatible with each other,
> >> furthermore, point is this - testing one option in isolation is broken.
> >
> > Ok. For the case that we were using, it don't matter at all.  But in
> > general, there "could" (it is only one cc-option call in all sources).
> >
> > Anthony, what do you preffer:
> > - revert the patch and add another one that changes += by :=
>
> No, I don't want to revert this patch and switch to :=.  += should
> work.  I don't understand why it doesn't.
>
> What I'd prefer is for someone to figure out the root cause of += not
> working for us.  If we can't, I'd like a big fat comment stating that
> it's a known deficiency and we'll move on.

"make" complains about an infinite recursive assignment because the first 
assignment of CFLAGS defines a recursively-expanded variable and the right 
value is a macro which uses the CFLAGS.

As explained in 
http://www.gnu.org/software/make/manual/html_node/Appending.html , it can be 
fixed by making CFLAGS a simply-expanded variable at its first assignment.

I have submitted a patch.
-- 
Thomas

[Qemu-devel] Re: [COMMIT 8a2e6ab] Remove CFLAGS parameter in cc-option

[Thomas Monjalon]

Juan Quintela wrote:
> Thomas Monjalon <thomas@monjalon.net> wrote:
> > Anthony Liguori wrote:
> >> What I'd prefer is for someone to figure out the root cause of += not
> >> working for us.  If we can't, I'd like a big fat comment stating that
> >> it's a known deficiency and we'll move on.
> >
> > "make" complains about an infinite recursive assignment because the first
> > assignment of CFLAGS defines a recursively-expanded variable and the
> > right value is a macro which uses the CFLAGS.
> >
> > As explained in
> > http://www.gnu.org/software/make/manual/html_node/Appending.html , it can
> > be fixed by making CFLAGS a simply-expanded variable at its first
> > assignment.
> >
> > I have submitted a patch.
>
> Thanks for the explanation.
> I haven't yet seen your patch, but I agree that right thing to do is
> do the 1st assignment as QEMU_CFLAGS := foo
>
> We can't really use CFLAGS, because CFLAGS need to work from the command
> line
>
> make CFLAGS="-O0 -g"
>
> or I am lossing something obvious?

In order to use the variable from the command line, we should use the 
syntax "override" before each CFLAGS modification. This way, it is possible 
to append options after the user ones.
If override is not used, "make" expand the variable by using the command line 
value in priority, which overwrites the Makefile value.

I could send another patch for that but it seems that my mails are filtered 
out by the mailing list.
-- 
Thomas