Bug 100028 - Debug information is incomplete or missing

[Andrew Haley]

Kelly O'Hair wrote:
> 
> Seems ok to me, just a few observations:
> 
> Do we need this:
> 
> 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

I think we probably do, yes.  I was trying to follow the principle of
touching as little as possible with my first patch, but it seems to me
that $(DEBUG_CLASSFILES) is the appropriate make variable to control
this.

> Not sure what happens in ant if a property is not set but used???.
> 
> On this:
> 
> # DEBUG_BINARIES overrides everything
> ifeq ($(DEBUG_BINARIES), true)
>   CFLAGS_REQUIRED += -g
>   DEBUG_FLAG = -g
> endif
> 
> I think the idea with isolating the -g to a variable was to allow for
> needing to globally set it to -gstabs or -g1 or some custom debug form for
> everyone. e.g. being able to do a top level 'make DEBUG_BINARIES=true
> DEBUG_FLAG=-g1'.

Thanks, I hadn't considered this possibility.

> So how about:
> 
> # DEBUG_BINARIES overrides everything, use full -g debug information
> ifeq ($(DEBUG_BINARIES), true)
>   DEBUG_FLAG = -g
>   CFLAGS_REQUIRED += $(DEBUG_FLAG)
> endif

OK.

> Otherwise it seems fine.
> 
> If you have patch files for each repository (hg diff > patch) I can
> run them through our test build system JPRT for you, and send you
> the results.

Right, will do.

Thanks,
Andrew.