[Xorp-hackers] [PATCH1/3] main SConstract bugs

[Ben Greear]

On 09/26/2011 09:43 AM, "Igor Maravić" wrote:
>> On 09/26/2011 01:32 AM, "Igor Maravić" wrote:
>>>> On 09/23/2011 06:43 AM, "Igor Maravić" wrote:
>>>>> When I did "scons install" rtrmngr was looking for .xrls files in folder /xrl/targets not in folder /xorp/xrls, so I changed that.
>>>>> Also I removed ; from if statements.
>>>>> Also only when value of 'optimize' is set to 'override', we should skip appending CFLAGS/CXXFLAGS.
>>>>> When it's value is 'no', CFLAGS/CXXFLAGS still have some value to be appended.
>>>>
>>>> Please break this into 3 different patches for these 3 different
>>>> things.
>>>>
>>>> For the .xrls part, what error did you see?  I don't see
>>>> any trouble with 'scons install' on my systems.  What command
>>>> are you using for the compile?
>>>>
>>>> Thanks,
>>>> Ben
>>>>
>>>> --
>>>> Ben Greear<greearb at candelatech.com>
>>>> Candela Technologies Inc  http://www.candelatech.com
>>>>
>>>>
>>>
>>> Only when value of 'optimize' is set to 'override', we should skip appending CFLAGS/CXXFLAGS.
>>> When it's value is 'no', CFLAGS/CXXFLAGS still have some value to be appended.
>>
>> This code is already in an if statement that checks for override:
>>
>> # If the user didn't override our default optimization, then
>> # sanitize user's CFLAGS/CXXFLAGS to not contain optimization options,
>> # and map to an appropriate GCC flag.
>> if not env['optimize'] == 'override':
>>
>> Are you sure your patch is needed?
>>
>> Thanks,
>> Ben
>>
>>>
>>> Signed-off-by: Igor Maravić<igorm at etf.rs>
>>> ---
>>>
>>> diff -upNr xorp.orig/SConstruct xorp/SConstruct
>>> --- xorp.orig/SConstruct        2011-09-12 20:23:12.000000000 +0200
>>> +++ xorp/SConstruct        2011-09-23 15:28:34.429947091 +0200
>>> @@ -713,7 +713,7 @@ if not env['optimize'] == 'override':
>>>                     'highest': '-O3',
>>>                     'size': '-Os' }
>>>        bigoflag = bigodict[env['optimize']]
>>> -    if not env['optimize'] == 'no':
>>> +    if not env['optimize'] == 'override':
>>>            env.AppendUnique(CFLAGS = [ bigoflag ])
>>>            env.AppendUnique(CXXFLAGS = [ bigoflag ])
>>
>>
>> --
>> Ben Greear<greearb at candelatech.com>
>> Candela Technologies Inc  http://www.candelatech.com
>>
>>
>
> 100 % sure.
>
>> # If the user didn't override our default optimization, then
>> # sanitize user's CFLAGS/CXXFLAGS to not contain optimization options,
>> # and map to an appropriate GCC flag.
>
> This is for the upper line, not for the line that I changed.
>
> Here is the compiler output when patch is not applied.
>
>
> g++ -o obj/i686-pc-linux-gnu/libxipc/xrl_pf_factory.os -c -g3 -Werror -W -Wall -Wwrite-strings -Wcast-qual -Wpointer-arith -Wcast-align -Woverloaded-virtual -ftemplate-depth-25 -pipe -fPIC -DXRL_PF=120 -D_FORTIFY_SOURCE=0 -Iobj/i686-pc-linux-gnu -I. -I. libxipc/xrl_pf_factory.cc
> g++ -o obj/i686-pc-linux-gnu/libxipc/xrl_pf_stcp.os -c -g3 -Werror -W -Wall -Wwrite-strings -Wcast-qual -Wpointer-arith -Wcast-align -Woverloaded-virtual -ftemplate-depth-25 -pipe -fPIC -DXRL_PF=120 -D_FORTIFY_SOURCE=0 -Iobj/i686-pc-linux-gnu -I. -I. libxipc/xrl_pf_stcp.cc
> g++ -o obj/i686-pc-linux-gnu/libxipc/xrl_pf_stcp_ph.os -c -g3 -Werror -W -Wall -Wwrite-strings -Wcast-qual -Wpointer-arith -Wcast-align -Woverloaded-virtual -ftemplate-depth-25 -pipe -fPIC -DXRL_PF=120 -D_FORTIFY_SOURCE=0 -Iobj/i686-pc-linux-gnu -I. -I. libxipc/xrl_pf_stcp_ph.cc
>
> As you can see there is no -O0 flag.

Please show me your exact compile commands:  scons ....
with and without your patch, and let me know why your change
fixes something.  I'm not that great with python, but I
don't see what you are trying to accomplish with your
change.

As far as I can tell, if optimize is 'override', then
the bigoflag will always be appended with your change,
and the if branch you are modifying will always evaluate
to 'true'.

Maybe we should just add a new optimize option 'none' and change
that inner if statement to check for none instead of 'no' ?

> I'l send you patches tomorrow, it's the end of work time in Serbia.
> Could you send me some instructions how to make git patches.
> I made patches with help of linux kernel official instructions.

Those instructions should work.

Generally:

Start with clean tree.
Make one patch's worth of changes.
git commit ...
Make more changes
git commit
...

Then, use git format-patch to create a patch series:

# I use something like this, for some kernel-tree patches:
git format-patch --stat -p --raw -o ~/tmp/patches/ --signoff --subject-prefix="PATCH" 
2bea038c52e88badbbd5b420c1de918f7f2579e9..ec0dd267bf7d08cb30e321e45a75fd40edd7e528

Then, use git send-email to send those patches.  Here's an example of my usage.
Obviously, change the email address(s) and patch name(s).

git send-email --to=linux-nfs at vger.kernel.org --to=linux-kernel at vger.kernel.org /home/greearb/tmp/patches/0001-nfs-Fix-xpt_ready-list-corruption.patch


You can try sending them to just yourself at first and make sure you can apply the
patches with 'git am' to a clean tree.  That is how I will apply your patches.

Thanks,
Ben


>
> BR
> Igor Maravić
>


-- 
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc  http://www.candelatech.com