Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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 Black, Steve Reinhardt, and Nathan Binkert. Summary (updated) --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Diffs (updated) - SConstruct 9f9e10967912 src/SConscript 9f9e10967912 src/arch/SConscript 9f9e10967912 src/arch/isa_parser.py 9f9e10967912 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 request. Anyway, here are the comments I would have entered: This update uses Nate's callable-object suggestion, which I think cleans things up a bit, and also adds colorization, including --color and --no-color options to force colors on or off (overriding the default behavior of using sys.stdout.isatty() to decide). I made a stab at a maize-and-blue color scheme, but I'm not really sold on it; suggestions for a better color scheme are welcome. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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 Black, Steve Reinhardt, and Nathan Binkert. Changes --- Added a screenshot to show color scheme. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Diffs - SConstruct 9f9e10967912 src/SConscript 9f9e10967912 src/arch/SConscript 9f9e10967912 src/arch/isa_parser.py 9f9e10967912 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Screenshots --- sample colorized output http://reviews.m5sim.org/r/366/s/1/ Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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 comments enough to hold you up if you don't want to deal with them at all. SConstruct http://reviews.m5sim.org/r/366/#comment847 Can you derive from object? I really hate classic classes as they make various things annoying and the cause is not always obvious. SConstruct http://reviews.m5sim.org/r/366/#comment851 I hate lines like this. I'll register my annoyance, but won't continue further discussion. SConstruct http://reviews.m5sim.org/r/366/#comment848 This is probably pedantic, but these could be @classmethod (and self should be cls) SConstruct http://reviews.m5sim.org/r/366/#comment849 99 seems like a lot and goes against the idea of being concise. I don't care much though. Would anyone object to [%-8s] instead? the right justified seems odd. SConstruct http://reviews.m5sim.org/r/366/#comment850 out of curiosity, can you do the following instead? main['CCCOMSTR'] = TRANSFORM(CC) Certainly, I am pretty sure that the parameter to MakeAction/Action could be the callable and not the string. (String substitution just slows down SCons) - Nathan On 2011-01-06 11:15:30, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-06 11:15:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Diffs - SConstruct 9f9e10967912 src/SConscript 9f9e10967912 src/arch/SConscript 9f9e10967912 src/arch/isa_parser.py 9f9e10967912 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Screenshots --- sample colorized output http://reviews.m5sim.org/r/366/s/1/ Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On 2011-01-06 11:42:18, Nathan Binkert wrote: SConstruct, line 379 http://reviews.m5sim.org/r/366/diff/3/?file=8481#file8481line379 Can you derive from object? I really hate classic classes as they make various things annoying and the cause is not always obvious. No problem, it was just an oversight. On 2011-01-06 11:42:18, Nathan Binkert wrote: SConstruct, line 418 http://reviews.m5sim.org/r/366/diff/3/?file=8481#file8481line418 99 seems like a lot and goes against the idea of being concise. I don't care much though. Would anyone object to [%-8s] instead? the right justified seems odd. The 99 is just a practical approximation of infinity, not meant to be hit in practice... the main idea is to give a parameter you can explicitly set if you don't want all (or any) of the sources listed, so I can cleanly replace the TRANSFORM_NOSRC and TRANSFORM_1SRC things I had in the earlier version. On 2011-01-06 11:42:18, Nathan Binkert wrote: SConstruct, line 481 http://reviews.m5sim.org/r/366/diff/3/?file=8481#file8481line481 out of curiosity, can you do the following instead? main['CCCOMSTR'] = TRANSFORM(CC) Certainly, I am pretty sure that the parameter to MakeAction/Action could be the callable and not the string. (String substitution just slows down SCons) TRANSFORM(CC) doesn't work, but Transform(CC) does. I had tried this before and it didn't work so I gave up on it, but I must have had some other error at the time. On 2011-01-06 11:42:18, Nathan Binkert wrote: SConstruct, line 406 http://reviews.m5sim.org/r/366/diff/3/?file=8481#file8481line406 This is probably pedantic, but these could be @classmethod (and self should be cls) Yea, I knew that, but I didn't see any point to it, so I figured why bother with the extra verbiage. If there's an advantage to it I'd be glad to do it though. The one thing I would like would be to define color() in a way where I could use it immediately, so I could do 'Arrow = color(arrow_color, - )' instead of the current expression, but I couldn't find a reasonable way to get that to work. On 2011-01-06 11:42:18, Nathan Binkert wrote: SConstruct, line 383 http://reviews.m5sim.org/r/366/diff/3/?file=8481#file8481line383 I hate lines like this. I'll register my annoyance, but won't continue further discussion. This is one of the few places I feel like lines like this are useful... this whole colorization thing already chews up way too many lines for the value it brings IMO :-). I'm also tempted to do this: def color_pfx(self, s): return self.color(self.pfx_color, s) def color_srcs(self, s): return self.color(self.srcs_color, s) def color_tgts(self, s): return self.color(self.tgts_color, s) just because I hate repetitive stuff chewing up vertical screen space. - Steve --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/#review632 --- On 2011-01-06 11:15:30, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-06 11:15:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Diffs - SConstruct 9f9e10967912 src/SConscript 9f9e10967912 src/arch/SConscript 9f9e10967912 src/arch/isa_parser.py 9f9e10967912 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Screenshots --- sample colorized output http://reviews.m5sim.org/r/366/s/1/ Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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:30, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-06 11:15:30) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Diffs - SConstruct 9f9e10967912 src/SConscript 9f9e10967912 src/arch/SConscript 9f9e10967912 src/arch/isa_parser.py 9f9e10967912 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Screenshots --- sample colorized output http://reviews.m5sim.org/r/366/s/1/ Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. /Updated 2011-01-06 11:15:30/ Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. Testing quick regressions pass Diffs * SConstruct (9f9e10967912) * src/SConscript (9f9e10967912) * src/arch/SConscript (9f9e10967912) * src/arch/isa_parser.py (9f9e10967912) View Diff http://reviews.m5sim.org/r/366/diff/ Screenshots sample colorized output http://reviews.m5sim.org/r/366/s/1/ ___ 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: show sources and targets when building.
On Tue, Jan 4, 2011 at 10:09 PM, nathan binkert n...@binkert.org 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 SCons command line. It actually crossed my mind earlier today to use colorization to distinguish the common prefix part of the source path from the source-specific part, as a way of eliminating the ambiguity of how to form the target. The isatty check would take care of the less problem, though... Actually less -r allows you to take the control characters, so the --color would override the isatty() == false and output the control codes anyway. Ah, got it, I misunderstood your comment. 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 called directly by the user so you can't add params anyway (unless there's a mechanism I'm unaware of). This is all overkill, but is a nice feature imho. True enough... Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 called directly by the user so you can't add params anyway (unless there's a mechanism I'm unaware of). No, I think all of the colorization should be in a single function. It's definitely a mechanism to allow transform to take an argument. So you change this: main['CCCOMSTR']= ' [ CC] $TRANSFORM' to this: main['CCCOMSTR']= ' $TRANSFORM(CC)' I had initially implemented the STRIP stuff like that and I don't remember exactly how it worked, but my recollection is that TRANSFORM(CC) was called, expected to return a callable, then that callable was called as TRANSFORM is now (I just created a class). All that said, I think that the RHS of this thing is also allowed to be a function and we don't have to use the SCons string substitution. You'd want to do the same callable object thing. Make sense? Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Mon, Jan 3, 2011 at 10:41 PM, nathan binkert n...@binkert.org 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 [ STRIP] ALPHA_SE/m5.fast: .unstripped - (2) It gives the wrong impression when targets are in a parent directory relative to the source: [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc instead of [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc I don't see these as deal-breakers, and I'm fine with leaving them as they are unless someone has a reasonable idea about how to address them, but I thought I'd point out that the ambiguities aren't purely theoretical. hmmm I guess I still like the ability to cut and paste, but the ambiguity is annoying (and there are bound to be others. I'll let you decide. For #2, you could do ../decoder.cc, for #1, if you want, you could detect that the RHS is empty and do the whole basename. These ideas might be overkill. (Don't you love how a simple idea always gets more complicated?) Yea, given the amount of time I've spent on this already I'm not inclined to add a lot of sophisticated handling for a few exceptional cases. #2 in particular doesn't seem that bad to me; it is ambiguous, but in a sort of reasonable way, and adding a bunch of path-handling code seems like overkill. For #1, the output is less intuitive with the blank RHS, and it's pretty easy to check that the target is the same length as the common prefix to know that there's a problem. The question is what to do about it... one simple possibility is just to reverse the arrow: [ STRIP] ALPHA_SE/m5.fast - .unstripped Or I could trim back to the path separator: [ STRIP] ALPHA_SE/m5.fast.unstripped - m5.fast Or trim to the nearest '.' or path sep: [ STRIP] ALPHA_SE/m5.fast.unstripped - .fast Any preferences? Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
[ 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.unstripped - m5.fast Or trim to the nearest '.' or path sep: [ STRIP] ALPHA_SE/m5.fast.unstripped - .fast Again, would this apply to all? if so, I'd probably opt for the last one since we don't need it in the majority of cases. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Tue, Jan 4, 2011 at 11:24 AM, nathan binkert n...@binkert.org 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, I was thinking just for this case (or strictly, just for the ones where the common prefix is equal to the entire target, which leaves the RHS empty with the current code). If you always reverse the arrow, you'll end up having the same problem in the other direction (e.g., for the python embed step where foo.py - foo.py.cc). Or I could trim back to the path separator: [ STRIP] ALPHA_SE/m5.fast.unstripped - m5.fast Or trim to the nearest '.' or path sep: [ STRIP] ALPHA_SE/m5.fast.unstripped - .fast Again, would this apply to all? if so, I'd probably opt for the last one since we don't need it in the majority of cases. The current code works 99% of the time... always trimming back to the nearest anything can cause issues; for example, with the swig lines: bar/foo.i - _wrap.cc, .py it looks nice as it is, but the common prefix is just bar/foo, so you don't want to unconditionally go back to look for a . or path sep. OTOH, I already back up a char when the common prefix ends in '.', otherwise you'd get foo.cc - o instead of foo.cc - .o so maybe I can find a way to generalize that which will take care of the .unstripped case. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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 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 - Ali On 2011-01-03 16:10:27, Steve Reinhardt wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-03 16:10:27) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Diffs - SConstruct 7338bc628489 src/SConscript 7338bc628489 src/arch/SConscript 7338bc628489 src/arch/isa_parser.py 7338bc628489
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 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. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Tue, Jan 4, 2011 at 5:44 PM, nathan binkert n...@binkert.org 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 = Blue + ' [ CC] ' + Yellow + '$TRANSFORMATION' + End 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 to use colorization to distinguish the common prefix part of the source path from the source-specific part, as a way of eliminating the ambiguity of how to form the target. The isatty check would take care of the less problem, though... Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 to use colorization to distinguish the common prefix part of the source path from the source-specific part, as a way of eliminating the ambiguity of how to form the target. The isatty check would take care of the less problem, though... Actually less -r allows you to take the control characters, so the --color would override the isatty() == false and output the control codes anyway. As a thought, TRANSFORM should probably take the action abbreviation (e.g. CC) as a parameter if you're going to do the colorization. This is all overkill, but is a nice feature imho. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: scons: show sources and targets when building.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Diffs - SConstruct 7338bc628489 src/SConscript 7338bc628489 src/arch/SConscript 7338bc628489 src/arch/isa_parser.py 7338bc628489 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Testing quick regressions pass Diffs * SConstruct (7338bc628489) * src/SConscript (7338bc628489) * src/arch/SConscript (7338bc628489) * src/arch/isa_parser.py (7338bc628489) View Diff http://reviews.m5sim.org/r/366/diff/ ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev ___ m5-dev mailing list
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- 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 Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Diffs (updated) - SConstruct 7338bc628489 src/SConscript 7338bc628489 src/arch/SConscript 7338bc628489 src/arch/isa_parser.py 7338bc628489 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 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 replacement. Example output for the new diff, corresponding to the previous example selection: [ CXX] ALPHA_SE/sim/main: .cc - .do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] - ALPHA_SE/base/traceflags.hh [ CXX] ALPHA_SE/python/swig/pyevent: .cc - .do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] - ALPHA_SE/arch/isa_traits.hh [ CXX] ALPHA_SE/python/swig/pyobject: .cc - .do [SWIG] ALPHA_SE/python/swig/core: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/swig/core_wrap: .cc - .do [SWIG] ALPHA_SE/python/swig/debug: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/swig/debug_wrap: .cc - .do ... [GENERATE] - ALPHA_SE/arch/vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] - ALPHA_SE/arch/types.hh [GENERATE] - ALPHA_SE/arch/registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha/: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base/remote_gdb: .cc - .do [ CXX] ALPHA_SE/base/socket: .cc - .do ... [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal/vptype_Process: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/m5/internal/vptype_Process_wrap: .cc - .do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal/vptype_AddrRange: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/m5/internal/vptype_AddrRange_wrap: .cc - .do ... [EMBED PY] ALPHA_SE/python/m5/internal/param_BaseSimpleCPU.py: - .cc [ CXX] ALPHA_SE/python/m5/internal/param_BaseSimpleCPU.py: .cc - .do [EMBED PY] ALPHA_SE/python/m5/internal/param_LiveProcess.py: - .cc [ CXX] ALPHA_SE/python/m5/internal/param_LiveProcess.py: .cc - .do [ TRACING] - ALPHA_SE/base/traceflags.py [EMBED PY] ALPHA_SE/base/traceflags.py: - .cc [ CXX] ALPHA_SE/base/traceflags.py: .cc - .do [ CXX] ALPHA_SE/base/date: .cc - .do [LINK] - ALPHA_SE/m5.debug ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 which one is compiling and (2) for swig, it prints the foo_wrap.cc target file name, which takes some (admittedly little) inside knowledge to map back to the foo.i file if you care, and doesn't mention the generated .py file. Anyway, it was just enough annoyance to lead me to think that the problem was inconsistently printing the source sometimes and the target other times... one simple solution would be just to consistently print one or the other, but since it's not obvious which one is more useful, I thought why not print both. And here I am. I think this version would be helpful in addressing the newbies-don't-know-where-generated-files-come-from problem too, if they just bother to look at the output while it builds. What don't you like about these newer versions? Just that they're more verbose? Steve On Mon, Jan 3, 2011 at 3:45 PM, Gabe Black gbl...@eecs.umich.edu wrote: 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. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal:
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 it every now and then to see if it finished or failed, but it was nice that it was relatively clean and sparse and would give you a quick idea of what it was chewing on. I also think every time we say if they just X we're forgetting that many people won't know or think to just X, so we shouldn't bend things around too far to accommodate that (not saying that's the case here, but still). For instance, Ali's links after warns, panics, etc. are a pretty good idea if people were to just follow them and put some info there, but I'd be willing to bet very few or even none of those links actually has anything. And actually speaking of which, we might want to get rid of those links. Gabe Steve Reinhardt wrote: 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 which one is compiling and (2) for swig, it prints the foo_wrap.cc target file name, which takes some (admittedly little) inside knowledge to map back to the foo.i file if you care, and doesn't mention the generated .py file. Anyway, it was just enough annoyance to lead me to think that the problem was inconsistently printing the source sometimes and the target other times... one simple solution would be just to consistently print one or the other, but since it's not obvious which one is more useful, I thought why not print both. And here I am. I think this version would be helpful in addressing the newbies-don't-know-where-generated-files-come-from problem too, if they just bother to look at the output while it builds. What don't you like about these newer versions? Just that they're more verbose? Steve On Mon, Jan 3, 2011 at 3:45 PM, Gabe Black gbl...@eecs.umich.edu mailto:gbl...@eecs.umich.edu wrote: 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. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 replacement. Example output for the new diff, corresponding to the previous example selection: 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 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 Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 this second diff as an alternative and not necessarily a replacement. Example output for the new diff, corresponding to the previous example selection: 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 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 I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 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 I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. I frequently compile many different versions of the source, and I like to know if it's working on the .do or the .fo (so I can get an idea of how far things are along.) If Gabe and Ali don't like this, we could go so far as to do Gabe's idea of VERBOSE=1 or VERBOSE=2, though that seems like overkill to me. I personally really do like this diff and would hate to see it go. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Mon, Jan 3, 2011 at 8:38 PM, nathan binkert n...@binkert.org 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.) Glad it's not just me :-). [ 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 decent idea... I didn't do it that way because it makes it potentially ambiguous about where the new extension gets spliced in, but in practice that's got to be rare, and having at least one file name you can cut and paste is a win (that's one thing I wasn't thrilled with about either of my patches). I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. There's a huge difference in verbosity and readability between this and verbose... if Ali's patch was 0 and this is a 1, the old verbose is like 18. Not to mention that this makes it clear what the sources and targets are, which is hard to do when switching around between different command lines for different tools. I frequently compile many different versions of the source, and I like to know if it's working on the .do or the .fo (so I can get an idea of how far things are along.) If Gabe and Ali don't like this, we could go so far as to do Gabe's idea of VERBOSE=1 or VERBOSE=2, though that seems like overkill to me. I personally really do like this diff and would hate to see it go. 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 probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. I'm fine with the change. Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Mon, Jan 3, 2011 at 8:57 PM, Steve Reinhardt ste...@gmail.com 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 decent idea... I didn't do it that way because it makes it potentially ambiguous about where the new extension gets spliced in, but in practice that's got to be rare, and having at least one file name you can cut and paste is a win (that's one thing I wasn't thrilled with about either of my patches). 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 impression when targets are in a parent directory relative to the source: [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc instead of [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc I don't see these as deal-breakers, and I'm fine with leaving them as they are unless someone has a reasonable idea about how to address them, but I thought I'd point out that the ambiguities aren't purely theoretical. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 really insist, it probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. I'm fine with the change. Ali Yeah, I'm probably ok with it too. I like the old version a bit better, but it's not a big deal if you guys really like the new way. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
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 impression when targets are in a parent directory relative to the source: [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc instead of [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc I don't see these as deal-breakers, and I'm fine with leaving them as they are unless someone has a reasonable idea about how to address them, but I thought I'd point out that the ambiguities aren't purely theoretical. hmmm I guess I still like the ability to cut and paste, but the ambiguity is annoying (and there are bound to be others. I'll let you decide. For #2, you could do ../decoder.cc, for #1, if you want, you could detect that the RHS is empty and do the whole basename. These ideas might be overkill. (Don't you love how a simple idea always gets more complicated?) Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
