Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Gabe Black
Hehe. http://www.imdb.com/title/tt0584442/quotes?qt0399229 Ali Saidi wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/366/ > > > How about leaving the tool name grey? > > Ali > > > - Ali > > > On January 6th, 2011, 11:15 a.m., Steve Reinhardt wr

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Ali Saidi
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/#review636 --- How about leaving the tool name grey? Ali - Ali On 2011-01-06 11:15:3

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Nathan Binkert
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/#review634 --- SConstruct Wow, we're h

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Steve Reinhardt
> On 2011-01-06 11:42:18, Nathan Binkert wrote: > > SConstruct, line 379 > > > > > > Can you derive from object? I really hate classic classes as they make > > various things annoying and the cause is not always obvious. No

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Nathan Binkert
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/#review632 --- Ship it! We've beaten this to death, so I don't care about any of my comm

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Steve Reinhardt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-06 11:15:30.932181) Review request for Default, Ali Saidi, Gabe Bl

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Steve Reinhardt
So is it possible to populate the "Changes" section with some comments when you update a patch using 'hg postreview -e NNN'? I've tried it with and without '-u' and each time it publishes the updated diff without giving me a chance to add/edit comments on the website like it does with the original

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-06 Thread Steve Reinhardt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-06 11:04:43.691088) Review request for Default, Ali Saidi, Gabe Bl

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-05 Thread nathan binkert
>> As a thought, TRANSFORM should probably take the action abbreviation >> (e.g. "CC") as a parameter if you're going to do the colorization. > > I don't follow... do you think the filenames should be colorized differently > depending on the command?  Note that the TRANSFORM* functions don't get >

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-05 Thread Steve Reinhardt
On Tue, Jan 4, 2011 at 10:09 PM, nathan binkert wrote: > >> Neat idea, but you need to check "sys.stdout.isatty()" and conditionally > >> add the color. Then you're going to want a way to override that if > you're > >> piping through less :) Seems like we could add a --color option to the > >>

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread nathan binkert
>> Neat idea, but you need to check "sys.stdout.isatty()" and conditionally >> add the color.  Then you're going to want a way to override that if you're >> piping through less :)  Seems like we could add a --color option to the >> SCons command line. > > It actually crossed my mind earlier today t

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread Steve Reinhardt
On Tue, Jan 4, 2011 at 5:44 PM, nathan binkert wrote: > I think we talked about this and I'm ok with what we ended up on the mailing > list... Bonus points if you colorize the output :)... >> Blue = '\033[94m' >> Yellow = '\033[94m' >> Green = '\033[92m' >> End = '\0330m' >> CCCOMSTR =

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread nathan binkert
> > I think we talked about this and I'm ok with what we ended up on the mailing > list... Bonus points if you colorize the output :)... > Blue = '\033[94m' > Yellow = '\033[94m' > Green = '\033[92m' > End = '\0330m' > CCCOMSTR = Blue + ' [ CC] ' + Yellow + '$TRANSFORMATION' + End > > Neat

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread Ali Saidi
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/#review620 --- Ship it! I think we talked about this and I'm ok with what we ended up on

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread Steve Reinhardt
On Tue, Jan 4, 2011 at 11:24 AM, nathan binkert wrote: > > [ STRIP] ALPHA_SE/m5.fast <- .unstripped > Would it be for just this one, or all? I think that we generally want > to cut and paste the source, not the target, so either way, this > doesn't seem like a great option. > For all of these

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread nathan binkert
>  [   STRIP] ALPHA_SE/m5.fast <- .unstripped Would it be for just this one, or all? I think that we generally want to cut and paste the source, not the target, so either way, this doesn't seem like a great option. > Or I could trim back to the path separator: >  [   STRIP] ALPHA_SE/m5.fast.unstri

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-04 Thread Steve Reinhardt
On Mon, Jan 3, 2011 at 10:41 PM, nathan binkert wrote: > > So far I've found two oddities with a straightforward implementation of > this > > approach: > > > > (1) It breaks down when the target is a prefix of the source: > > > > [ STRIP] ALPHA_SE/m5.fast.unstripped -> > > > > instead of > > >

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread nathan binkert
> So far I've found two oddities with a straightforward implementation of this > approach: > > (1) It breaks down when the target is a prefix of the source: > >  [   STRIP] ALPHA_SE/m5.fast.unstripped -> > > instead of > >  [   STRIP] ALPHA_SE/m5.fast: .unstripped -> > > (2) It gives the wrong impr

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Gabe Black
Ali Saidi wrote: > > On Jan 3, 2011, at 10:57 PM, Steve Reinhardt wrote: >> >> >> >> With Nate's suggested change, for CXX lines we're really just tacking >> on the ' -> .o' at the end... the initial part of the line is >> unchanged, and the number of extra chars per line is 6 or 7. If you >> guys

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Steve Reinhardt
On Mon, Jan 3, 2011 at 8:57 PM, Steve Reinhardt wrote: > > >>> [ CXX] ALPHA_SE/sim/main: .cc -> .do >> >> My only change would be to remove the colon space. It serves little >> >> purpose and makes cut-and-paste less useful. >> >> >> >> [ CXX] ALPHA_SE/sim/main.cc -> .do >> > > That's a

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Ali Saidi
On Jan 3, 2011, at 10:57 PM, Steve Reinhardt wrote: > > > With Nate's suggested change, for CXX lines we're really just tacking on the > ' -> .o' at the end... the initial part of the line is unchanged, and the > number of extra chars per line is 6 or 7. If you guys really insist, it > proba

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Steve Reinhardt
On Mon, Jan 3, 2011 at 8:38 PM, nathan binkert wrote: > >> I love this and I've been wanting it for a while for your reason #1, > >> also, there are some ambiguous things where the same file is printed > >> because one is a TARGET and another thing uses a SOURCE (that always > >> bothered me.) >

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread nathan binkert
>> I love this and I've been wanting it for a while for your reason #1, >> also, there are some ambiguous things where the same file is printed >> because one is a TARGET and another thing uses a SOURCE (that always >> bothered me.) >> >>>  [     CXX] ALPHA_SE/sim/main: .cc -> .do >> My only change

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Ali Saidi
On Jan 3, 2011, at 10:21 PM, nathan binkert wrote: >> I had originally been more aggressive about common prefix extraction, but >> there were some glitches that made me go with the path-based approach... >> however, in the process of uploading this diff I thought of some fixes. So >> consider th

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread nathan binkert
> I had originally been more aggressive about common prefix extraction, but > there were some glitches that made me go with the path-based approach... > however, in the process of uploading this diff I thought of some fixes.  So > consider this second diff as an alternative and not necessarily a >

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Gabe Black
Yeah, primarily. In this new version there's enough text that it gets hard to pick out the nugget that lets you know what it's doing. We have some way to turn on the command lines again, don't we? I don't really remember off hand. Granted when things are compiling I usually just ignore it and check

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Steve Reinhardt
I don't recall all the details about what got me going on this, but I think a couple of the specific things I didn't like were (1) if you compile multiple binaries at the same time (e.g., m5.debug and m5.opt) the CXX lines print the source file which doesn't give you enough information to know whic

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Steve Reinhardt
I think this is the first time I've updated an existing diff via reviewboard, and I thought it would give me a chance to enter some comments on the website before publishing it like a new diff, but it didn't. so here are the comments that go with this diff: I had originally been more aggressive ab

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Steve Reinhardt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-03 16:10:27.987336) Review request for Default, Ali Saidi, Gabe Bl

Re: [m5-dev] Review Request: scons: show sources and targets when building.

2011-01-03 Thread Gabe Black
I like the original (Ali's version) better. When was it confusing? Perhaps there's a particular action that would be better the other way around. Maybe we should have graduated levels of verbosity for this stuff where, for example, 0 is Ali's version, 1 is this version, and 2 is the full command.