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

[Ben Greear]

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