Couple of comments.
> +# Generate hginfo.cc
> +# Anything we pass as a source gets setup as a dependence so rather than
> +# passing the SConscript/hg dir/etc we just squirrel away the SConstruct
> +# directory in the environment and retrieve it later. This seems to
> +# be the only reliable way to get the information if we're building in
> +# a directory outside of the m5 directory
> +env['SConstructDir'] = str(SCons.Node.FS.default_fs.SConstruct_dir)
> +env.Command('base/hginfo.cc', None, generate.hgInfo)
> +
Don't use your this environment variable hack. Instead, pass it as a
source, but wrap it with Value(). Value basically stores only strings
and if the string changes, causes the dependency to be regenerated.
Make sense? You'll see me use it all over src/Sconscript.
> + # Abuse the SCons dependence code to make the generation
> + # of hginfo.cc dependend on all the other cc files and the
> + # compiling of hginfo.cc dependent on all the objects
> + # but hginfo.o
> + hg_obj = env.Object('base/hginfo.cc')
> + env.Depends('base/hginfo.cc', sources)
> env.Depends(date_obj, objs)
> - objs.append(date_obj)
> + env.Depends(hg_obj, objs)
> + objs.extend([date_obj,hg_obj])
> return objs
I don't quite know that this qualifies as abuse, but it works :)
> + def hgInfo(self, target, source, env):
> + try:
This try block feels way too big and seems like it could hide some
lurking Exceptions. Can you create a sub function that generates the
file with the two parameters, and call it with unknown? e.g.
try:
import mercurial...
except ImportError:
generateFile("unknown", "unknown")
return
This would also force you to have the generation code in only one
place. That way, if we ever add variables or something, we won't
accidentally have two different versions.
> + # The SConscript squirrels away the SConstructDir variable in the
> + # env for us. We can't pass it as a source parameter because that
> + # would setup a depedence between everything in the directory and
> + # above and this file.
> +
> + scons_dir = env['SConstructDir']
Again, this hack is not necessary.
> + import mercurial.hg, mercurial.ui, mercurial.util,
> mercurial.node
> + if not exists(scons_dir) or not isdir(scons_dir) or \
> + not exists(join(scons_dir, ".hg")):
> + raise ValueError
Seems like you should check for the existence of .hg before you try to
import mercurial.
> + repo = mercurial.hg.repository(mercurial.ui.ui(), scons_dir)
> + rev = mercurial.node.nullrev + repo.changelog.count()
> + changenode = repo.changelog.node(rev)
> + changes = repo.changelog.read(changenode)
> + date = mercurial.util.datestr(changes[2])
> +
> + hg_stats = file(str(target[0]), 'w')
> + print >>hg_stats, 'const char *hgRev = "%s:%s";' % (rev,
> mercurial.node.hex(changenode))
> + print >>hg_stats, 'const char *hgDate = "%s";' % date
> + hg_stats.close()
> + mercurial.demandimport.disable()
> + except ImportError:
> + pass
> + except:
> + hg_stats = file(str(target[0]), 'w')
> + print >>hg_stats, 'const char *hgRev = "Unknown";'
> + print >>hg_stats, 'const char *hgDate = "Unknown";'
> + hg_stats.close()
> + mercurial.demandimport.disable()
Why do you do this at the end? I'm not clear on the demandimport
stuff, but it seems that you may as well just do it as soon as you've
imported mercurial.
> +
> +
> +
> +
I object to random added whitespace.
> diff --git a/src/python/m5/main.py b/src/python/m5/main.py
> --- a/src/python/m5/main.py
> +++ b/src/python/m5/main.py
> @@ -269,6 +269,10 @@ def main():
> print "M5 compiled %s" % internal.core.cvar.compileDate;
> print "M5 started %s" % datetime.datetime.now().ctime()
> print "M5 executing on %s" % socket.gethostname()
> +
> + print "M5 revision %s" % internal.core.cvar.hgRev
> + print "M5 commit date %s" % internal.core.cvar.hgDate
> +
> print "command line:",
> for argv in sys.argv:
> print argv,
> diff --git a/src/python/swig/core.i b/src/python/swig/core.i
> --- a/src/python/swig/core.i
> +++ b/src/python/swig/core.i
> @@ -39,6 +39,8 @@
> #include "sim/startup.hh"
>
> extern const char *compileDate;
> +extern const char *hgRev;
> +extern const char *hgDate;
> %}
>
> %include "stdint.i"
> @@ -51,6 +53,8 @@ void doExitCleanup();
> void doExitCleanup();
>
> char *compileDate;
> +char *hgRev;
> +char *hgDate;
>
> void setClockFrequency(Tick ticksPerSecond);
Seems all good to me.
I like this idea. Are there any other variables that would be nice to
add? Versions of gcc and libraries and such?
Nate
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
