Closed
Bug 1074008
Opened 10 years ago
Closed 10 years ago
Add a --fix-stacks option to dmd.py
Categories
(Core :: DMD, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
5.16 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
After bug 1044709 converts DMD to produce JSON output, on Linux and Mac you'll have to run the appropriate fix*.py script *and* dmd.py to get nice output, which is a pain. My plan is to add a --fix-stacks option to dmd.py that would cause the JSON to be fixed in-place. (If the output had already been fixed, this would have no effect.)
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8496740 -
Flags: review?(mh+mozilla)
Comment 2•10 years ago
|
||
Comment on attachment 8496740 [details] [diff] [review] Add a --fix-stacks option to dmd.py Review of attachment 8496740 [details] [diff] [review]: ----------------------------------------------------------------- ::: memory/replace/dmd/dmd.py @@ +173,5 @@ > > +# Fix stacks if necessary: first write the output to a tempfile, then replace > +# the original file with it. > +def maybeFixStackTraces(): > + if args.fix_stacks: That should be tested outside (and the function renamed not to start with maybe as a consequence) @@ +194,5 @@ > + # Fix stacks, writing output to a temporary file, and then overwrite > + # the original file. > + with tempfile.NamedTemporaryFile(delete=False) as tmp: > + for line in args.input_file: > + tmp.write(fixModule.fixSymbols(line)) Seems to me you intended to write tmp.write(fix(line)) but forgot.
Attachment #8496740 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8496740 [details] [diff] [review] Add a --fix-stacks option to dmd.py Review of attachment 8496740 [details] [diff] [review]: ----------------------------------------------------------------- ::: memory/replace/dmd/dmd.py @@ +173,5 @@ > > +# Fix stacks if necessary: first write the output to a tempfile, then replace > +# the original file with it. > +def maybeFixStackTraces(): > + if args.fix_stacks: I did it that way to mirror |maybeTrimStackTraces|, which involves two options and so can't be done that way.
Assignee | ||
Comment 4•10 years ago
|
||
erahm and mccr8 suggested that the JSON include a property that describes whether stacks have been fixed. It's a nice idea but there's a problem: B2G. The B2G stack fixing is done by get_about_memory.py independently of dmd.py. So it would be easy to end up with a file that claims to not have been fixed but actually has been fixed. All of which makes me nervous.
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8496740 [details] [diff] [review] Add a --fix-stacks option to dmd.py Review of attachment 8496740 [details] [diff] [review]: ----------------------------------------------------------------- ::: memory/replace/dmd/dmd.py @@ +182,5 @@ > + # XXX: should incorporate fix_stack_using_bpsyms.py here as well, like > + # in testing/mochitests/runtests.py > + sysname = platform.system() > + if sysname == 'Linux': > + import fix_linux_stack as fixModule I need to get fix_stack_using_bpsyms.py working in order to make the xpcshell test (bug 1073312) work on TBPL. And that requires knowing the symbolsPath. glandium, do you know how I can get the symbolsPath that in this script? Looking around, it looks like I can get it something like this: > options.symbolsPath = os.path.join(distdir, 'crashreporter-symbols') and |distdir| like this: > distdir = os.path.join(topobjdir, 'dist') but I don't know how to get topobjdir in this script. Or maybe all that is totally heading in the wrong direction, I don't know.
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 6•10 years ago
|
||
> I need to get fix_stack_using_bpsyms.py working in order to make the
> xpcshell test (bug 1073312) work on TBPL. And that requires knowing the
> symbolsPath.
Another possibility: the xpcshell test harness knows the symbolsPath. It already sets a couple of environment variables ($DMD, $PYTHON) to pass information to DMD's xpcshell test. It could set another one to pass the symbolsPath.
Comment 7•10 years ago
|
||
Comment 6 is probably better than munging paths in crazy ways, especially when the test environment on local builds and tbpl builds is so completely different.
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #4) > erahm and mccr8 suggested that the JSON include a property that describes > whether stacks have been fixed. It's a nice idea but there's a problem: B2G. > The B2G stack fixing is done by get_about_memory.py independently of dmd.py. > So it would be easy to end up with a file that claims to not have been fixed > but actually has been fixed. All of which makes me nervous. But I will make stack-fixing in dmd.py the default behaviour.
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b73787681bd
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2b73787681bd
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•