On Apr 17, 2009, at 9:11 AM, Andrew Haley wrote:
Jonathan Gibbons wrote:
It's one thing to be enabling more debug information if you set
flags,
and/or making it more consistent, but I'm not happy with you removing
debug information by default. At least from reading the changeset
diff,
it seems there is no way to get what was previously the default
amount
of debug info, which is source,lines (but not vars).
Source and lines are good to get useful stack traces in the face of
exceptions, even if var information is not present. var
information is
typically not required in product builds.
Oh dear. I discussed this at considerable length with Kelly, whose
suggestion it was to change
ifeq ($(DEBUG_CLASSFILES), true)
ANT_OPTIONS += -Djavac.debug=true
ANT_OPTIONS += -Djavac.debuglevel=source,lines,vars
endif
to be
ifeq ($(DEBUG_CLASSFILES), true)
ANT_OPTIONS += -Djavac.debug=true
ANT_OPTIONS += -Djavac.debuglevel=source,lines,vars
else
ANT_OPTIONS += -Djavac.debug=false
ANT_OPTIONS += -Djavac.debuglevel=
endif
It was not my intention to break anything that already existed, but
simply to add debuginfo where it was disabled.
Can you please point me to exactly which hunks of my diff are
problematic?
The only place I can see that might be a problem is javac.
If your intention was to ensure that debug info was added, then the
changeset
for langtools (which you summarise above) would not seem to be
necessary,
because the old code honored setting DEBUG_CLASSFILES to true. It is
the new "else" branch that disables debug info if DEBUG_CLASSFILES is
not set that is problematic.
I accept this changeset is part of an overall larger set of changesets
across
all repositories. What exactly were the goals? It seems to me it is
arguably
desirable to have three settings (not two).
All debugging on (source,lines,vars)
Basic debugging on (source,lines)
No debugging
There is arguably a fourth candidate for consideration, which is
"local default"
which allows the choice to be made on a more fine-grained (albeit
inconsistent)
basis.
-- Jon
Thanks,
Andrew.
