Re: [m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-25 Thread Nathan Binkert

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/604/#review1015
---

Ship it!


I didn't try it, but i think I understand the implications and I think it looks 
alright.  I say go for it.  I'm sure you can fix it quickly if it goes sour.


SConstruct


Ok.  I didn't notice that.


- Nathan


On 2011-03-24 08:11:13, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/604/
> ---
> 
> (Updated 2011-03-24 08:11:13)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> scons: interpret paths relative to launch directory
> 
> Make sure all command-line targets and EXTRAS directories
> are interpreted relative to the launch directory.  This
> turns out to be very useful when building code from an
> EXTRAS directory using SCons's -C option.
> 
> We were trying to do this with targets but it didn't actually
> work since we didn't update BUILD_TARGETS (so SCons got
> confused internally).  We weren't even trying with EXTRAS.
> 
> To simplify the code, the default target is also interpreted
> relative to the launch dir even though it was explicitly
> handled as relative to the m5 dir before... I doubt anyone
> really uses this anyway so it didn't seem worth the complexity.
> (Maybe we should get rid of it?)
> 
> 
> Diffs
> -
> 
>   SConstruct 89cd8302abd3 
> 
> Diff: http://reviews.m5sim.org/r/604/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev


Re: [m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-25 Thread Steve Reinhardt


> On 2011-03-24 11:15:10, Nathan Binkert wrote:
> > SConstruct, line 366
> > 
> >
> > Why did you get rid of the validators?  My recollection is that a typo 
> > in EXTRAS can be baffling.

I didn't get rid of the check, just moved it (see the added code at the bottom, 
line 794 in the new file).

I stopped using the scons converter/validator hooks because I wanted to reuse 
the converter code for the targets, and scons didn't seem to like passing lists 
around instead of strings.  Plus it's poorly documented and we don't use those 
hooks for any other vars so it seemed cleaner just to abandon it entirely.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/604/#review1012
---


On 2011-03-24 08:11:13, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/604/
> ---
> 
> (Updated 2011-03-24 08:11:13)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> scons: interpret paths relative to launch directory
> 
> Make sure all command-line targets and EXTRAS directories
> are interpreted relative to the launch directory.  This
> turns out to be very useful when building code from an
> EXTRAS directory using SCons's -C option.
> 
> We were trying to do this with targets but it didn't actually
> work since we didn't update BUILD_TARGETS (so SCons got
> confused internally).  We weren't even trying with EXTRAS.
> 
> To simplify the code, the default target is also interpreted
> relative to the launch dir even though it was explicitly
> handled as relative to the m5 dir before... I doubt anyone
> really uses this anyway so it didn't seem worth the complexity.
> (Maybe we should get rid of it?)
> 
> 
> Diffs
> -
> 
>   SConstruct 89cd8302abd3 
> 
> Diff: http://reviews.m5sim.org/r/604/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev


Re: [m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-24 Thread Nathan Binkert

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/604/#review1012
---



SConstruct


Why did you get rid of the validators?  My recollection is that a typo in 
EXTRAS can be baffling.


- Nathan


On 2011-03-24 08:11:13, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/604/
> ---
> 
> (Updated 2011-03-24 08:11:13)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> scons: interpret paths relative to launch directory
> 
> Make sure all command-line targets and EXTRAS directories
> are interpreted relative to the launch directory.  This
> turns out to be very useful when building code from an
> EXTRAS directory using SCons's -C option.
> 
> We were trying to do this with targets but it didn't actually
> work since we didn't update BUILD_TARGETS (so SCons got
> confused internally).  We weren't even trying with EXTRAS.
> 
> To simplify the code, the default target is also interpreted
> relative to the launch dir even though it was explicitly
> handled as relative to the m5 dir before... I doubt anyone
> really uses this anyway so it didn't seem worth the complexity.
> (Maybe we should get rid of it?)
> 
> 
> Diffs
> -
> 
>   SConstruct 89cd8302abd3 
> 
> Diff: http://reviews.m5sim.org/r/604/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev


Re: [m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-24 Thread Steve Reinhardt
If you always invoke scons from the m5 directory, this will change
nothing.  It should only matter if you use the -C, -D, -f, -u, or -U
options to scons, since I think those are the only ways to invoke
scons on m5/SConstruct without being in that directory.

The default target is just the target that gets built if you don't
specify one explicitly:
http://repo.m5sim.org/m5/file/ec1eecca2f8f/SConstruct#l257

Steve

On Thu, Mar 24, 2011 at 10:46 AM, Gabe Black  wrote:
> Could you please summarize in practical terms what this is changing? How
> will this affect me when I go to invoke scons? Also, what's this default
> target nobody uses?
>
> Gabe
>
> On 03/24/11 11:11, Steve Reinhardt wrote:
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/604/
>> ---
>>
>> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
>> Nathan Binkert.
>>
>>
>> Summary
>> ---
>>
>> scons: interpret paths relative to launch directory
>>
>> Make sure all command-line targets and EXTRAS directories
>> are interpreted relative to the launch directory.  This
>> turns out to be very useful when building code from an
>> EXTRAS directory using SCons's -C option.
>>
>> We were trying to do this with targets but it didn't actually
>> work since we didn't update BUILD_TARGETS (so SCons got
>> confused internally).  We weren't even trying with EXTRAS.
>>
>> To simplify the code, the default target is also interpreted
>> relative to the launch dir even though it was explicitly
>> handled as relative to the m5 dir before... I doubt anyone
>> really uses this anyway so it didn't seem worth the complexity.
>> (Maybe we should get rid of it?)
>>
>>
>> Diffs
>> -
>>
>>   SConstruct 89cd8302abd3
>>
>> Diff: http://reviews.m5sim.org/r/604/diff
>>
>>
>> Testing
>> ---
>>
>>
>> Thanks,
>>
>> Steve
>>
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev


Re: [m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-24 Thread Gabe Black
Could you please summarize in practical terms what this is changing? How
will this affect me when I go to invoke scons? Also, what's this default
target nobody uses?

Gabe

On 03/24/11 11:11, Steve Reinhardt wrote:
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/604/
> ---
>
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
>
>
> Summary
> ---
>
> scons: interpret paths relative to launch directory
>
> Make sure all command-line targets and EXTRAS directories
> are interpreted relative to the launch directory.  This
> turns out to be very useful when building code from an
> EXTRAS directory using SCons's -C option.
>
> We were trying to do this with targets but it didn't actually
> work since we didn't update BUILD_TARGETS (so SCons got
> confused internally).  We weren't even trying with EXTRAS.
>
> To simplify the code, the default target is also interpreted
> relative to the launch dir even though it was explicitly
> handled as relative to the m5 dir before... I doubt anyone
> really uses this anyway so it didn't seem worth the complexity.
> (Maybe we should get rid of it?)
>
>
> Diffs
> -
>
>   SConstruct 89cd8302abd3 
>
> Diff: http://reviews.m5sim.org/r/604/diff
>
>
> Testing
> ---
>
>
> Thanks,
>
> Steve
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev


[m5-dev] Review Request: scons: interpret paths relative to launch directory

2011-03-24 Thread Steve Reinhardt

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/604/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

scons: interpret paths relative to launch directory

Make sure all command-line targets and EXTRAS directories
are interpreted relative to the launch directory.  This
turns out to be very useful when building code from an
EXTRAS directory using SCons's -C option.

We were trying to do this with targets but it didn't actually
work since we didn't update BUILD_TARGETS (so SCons got
confused internally).  We weren't even trying with EXTRAS.

To simplify the code, the default target is also interpreted
relative to the launch dir even though it was explicitly
handled as relative to the m5 dir before... I doubt anyone
really uses this anyway so it didn't seem worth the complexity.
(Maybe we should get rid of it?)


Diffs
-

  SConstruct 89cd8302abd3 

Diff: http://reviews.m5sim.org/r/604/diff


Testing
---


Thanks,

Steve

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev