> On 2010-07-05 21:01:59, Steve Reinhardt wrote: > > src/SConscript, line 137 > > <http://reviews.m5sim.org/r/29/diff/1/?file=625#file625line137> > > > > What is srcpath used for? I see it getting set here and then passed > > around a lot but I don't see it getting used. > > Nathan Binkert wrote: > Actually, I was going to support an override where a path was specified > and the relative path would then be appended, but I thought that it was > overkill to do that. I could remove this code. This brings up another > question. Right now, when you get an error message, that message uses > arcname as the filename. arcname is basically the path that the file would > be if it were in a zip archive, and it has no real relationship to where the > file came from this seems to have confused people in the past. We could > instead use something like srcname (with the target name if there is no > source file). What do you think?
If using srcname in error messages makes more sense, that's fine by me. To the extent that srcpath isn't being used though, I'd get rid of it. Not to get too far off topic, but python error handling in general could be cleaned up... there are cases where you run into a straightforward problem, sometimes even with a decent error message, but an exception gets raised and we get a stack backtrace that totally overwhelms the error message. Seems like we should try to catch some common exceptions at the top level to avoid that. If we did, then maybe we could move toward using exceptions to flag all kinds of errors rather than trying to duplicate fatal() and panic() in python. > On 2010-07-05 21:01:59, Steve Reinhardt wrote: > > src/python/importer.py, line 64 > > <http://reviews.m5sim.org/r/29/diff/1/?file=626#file626line64> > > > > I think a more descriptive/specific env var is called for... how about > > M5_USE_PY_SOURCE? > > Nathan Binkert wrote: > M5_OVERRIDE_PY_SOURCE? I'm not sure yours really explains what's going > on either. I should clearly add a comment somewhere. I could also hack this > in as a command line option, though that'd mean doing some command line > parsing in C++, which may not be so bad since I could just search argv for > the specific argument. (The argument could be parsed in C++, but it would > also be a no-op argument in python and would get a help string as a result.) It explains it to me since I think of the "source" as the original source file/tree, and not some compressed copy squirreled away somewhere, but I can see if you take a broader view of "source" as anything that's not compiled then maybe it's still not too specific. I'm not convinced your idea is a lot better, since it doesn't explain what's overriding what. How about M5_OVERRIDE_EMBEDDED_PYTHON_ARCHIVE_WITH_FILES_FROM_SOURCE_TREE? :-) I'm fine with the env var, it's just a developer hack as you say, no need to get too crazy with the code. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/29/#review59 ----------------------------------------------------------- On 2010-07-05 17:25:00, Nathan Binkert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/29/ > ----------------------------------------------------------- > > (Updated 2010-07-05 17:25:00) > > > Review request for Default. > > > Summary > ------- > > python: Add mechanism to override code compiled into the exectuable > If the user sets the environment variable M5_OVERRIDE to True, then > imports that would normally find python code compiled into the executable > will instead first check in the absolute location where the code was > found during the build of the executable. This only works for files > in the src (or extras) directories, not automatically generated files. > > This is a developer feature! > > > Diffs > ----- > > src/SConscript 1b1f8f32fe86 > src/python/importer.py 1b1f8f32fe86 > src/sim/init.hh 1b1f8f32fe86 > src/sim/init.cc 1b1f8f32fe86 > > Diff: http://reviews.m5sim.org/r/29/diff > > > Testing > ------- > > > Thanks, > > Nathan > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
