Proposal to stop defining DEBUG_<person>
Kyle Huey
Mozilla. I propose that we stop defining this and either remove
everything that is #ifdef DEBUG_<username> or convert it to be #ifdef
DEBUG_<module> or #ifdef DEBUG_<feature>. Either the code in question
is useful enough to anyone hacking the module/feature that it should
be there for everyone or it probably doesn't belong in the tree. The
immediate motivation for doing this is that it will make the work
dholbert is doing in Bug 557566 to make warnings fatal in directories
known to be warning-free easier.
My plan of action is to turn this off and wait several weeks for code
that is still loved to be converted to a more appropriate #ifdef
before going through and removing whatever remains. There are no more
than 500 uses of this in the tree, so it shouldn't be terribly
painful. I'd be willing to convert them myself if the people who
"own" them can tell me what they should be changed to.
Any objections?
Justin Wood (Callek)
> or it probably doesn't belong in the tree. The
> immediate motivation for doing this is that it will make the work
> dholbert is doing in Bug 557566 to make warnings fatal in directories
> known to be warning-free easier.
>
> My plan of action is to turn this off and wait several weeks for code
> that is still loved to be converted to a more appropriate #ifdef
> before going through and removing whatever remains. There are no more
> than 500 uses of this in the tree, so it shouldn't be terribly
> painful. I'd be willing to convert them myself if the people who
> "own" them can tell me what they should be chan
I have long wished to see this abolished, but have never had the
motivation to do anything about it, including post. So I'm all for it!
--
~Justin Wood (Callek)
L. David Baron
> My plan of action is to turn this off and wait several weeks for code
> that is still loved to be converted to a more appropriate #ifdef
> before going through and removing whatever remains. There are no more
> than 500 uses of this in the tree, so it shouldn't be terribly
> painful. I'd be willing to convert them myself if the people who
> "own" them can tell me what they should be changed to.
Why not just switch off the build system defining them and leave
the code there to be resurrected as the owners of the code see fit?
-David
--
L. David Baron http://dbaron.org/
Mozilla Corporation http://www.mozilla.com/
Kyle Huey
fine with the more conservative approach.
- Kyle
On Sun, Apr 25, 2010 at 10:03 PM, L. David Baron <dba...@dbaron.org> wrote:
> On Sunday 2010-04-25 16:21 -0700, Kyle Huey wrote:
> > My plan of action is to turn this off and wait several weeks for code
> > that is still loved to be converted to a more appropriate #ifdef
> > before going through and removing whatever remains. There are no more
> > than 500 uses of this in the tree, so it shouldn't be terribly
> > painful. I'd be willing to convert them myself if the people who
> > "own" them can tell me what they should be changed to.
>
> Why not just switch off the build system defining them and leave
> the code there to be resurrected as the owners of the code see fit?
>
> -David
>
> --
> L. David Baron http://dbaron.org/
> Mozilla Corporation http://www.mozilla.com/
> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
Smaug
Please don't remove any DEBUG_smaug ifdefs, at least not right now.
I really need those, pretty much all.
I could convert them to DEBUG_cycle_collector_runs,
DEBUG_dom_range (or just remove the one related to DOMRanges),
DEBUG_window_activation and DEBUG_native_anonymous_event_propagation
or something like that.
Is there a bug open for this all?
-Olli
Kyle Huey
> Is there a bug open for this all?
>
I filed bug 561674 <https://bugzilla.mozilla.org/show_bug.cgi?id=561674>.
L. David Baron
> No real reason except a desire to remove cruft eventually. I'm perfectly
> fine with the more conservative approach.
I think I'd prefer it; there's a good bit of DEBUG_foo code that's
quite useful for people actually hacking on the code in question.
Some of it could be "trivially" rewritten in 5-20 minutes, but why
rewrite it every time? It's easier to fix any compilation errors
that creep in occasionally.
Smaug
> On Mon, Apr 26, 2010 at 11:40 AM, Smaug<sm...@welho.com> wrote:
>
>> I really need those, pretty much all.
>> I could convert them to DEBUG_cycle_collector_runs,
>> DEBUG_dom_range (or just remove the one related to DOMRanges),
>> DEBUG_window_activation and DEBUG_native_anonymous_event_propagation
>> or something like that.
>>
>
> repository do the same job?
>
> -Rob
One thing, I don't use mq.
And secondly, I'd have to keep those patches in all my (~20) trees and
update all the patches when something breaks or if I need to
add/remove something in them.
I'd like to avoid that and just keep them in one place and update
my local repos when needed.
-Olli
L. David Baron
> On Mon, Apr 26, 2010 at 11:40 AM, Smaug <sm...@welho.com> wrote:
>
> > I really need those, pretty much all.
> > I could convert them to DEBUG_cycle_collector_runs,
> > DEBUG_dom_range (or just remove the one related to DOMRanges),
> > DEBUG_window_activation and DEBUG_native_anonymous_event_propagation
> > or something like that.
> >
>
> repository do the same job?
But they're probably useful for someone else who comes along to work
on the same code, which is a good reason to have them in the tree.
Blair McBride
>> If they're only defined for you, wouldn't a set of mq patches in your local
>> > repository do the same job?
> But they're probably useful for someone else who comes along to work
> on the same code, which is a good reason to have them in the tree.
That's also a good reason to have them named something more appropriate
to the module/file/usage (ie, not a user name).
- Blair
Zack Weinberg
> Currently the build system defines DEBUG_<username> when building
> Mozilla. I propose that we stop defining this and either remove
> everything that is #ifdef DEBUG_<username> or convert it to be #ifdef
> DEBUG_<module> or #ifdef DEBUG_<feature>.
...
> Any objections?
I don't exactly *object*, but ... I use DEBUG_zack to wrap temporary
debugging code that I do *not* want to publish. This makes it easy to
verify that I've remembered to take it all out again, and if I do
push some of it to a shared tree by mistake, it doesn't affect anyone
else. I'd like to continue to have something that serves this function.
zw
Kyle Huey
>
> I don't exactly *object*, but ... I use DEBUG_zack to wrap temporary
> debugging code that I do *not* want to publish. This makes it easy to
> verify that I've remembered to take it all out again, and if I do
> push some of it to a shared tree by mistake, it doesn't affect anyone
> else. I'd like to continue to have something that serves this function.
>
> zw
Wouldn't defining DEBUG_zack in your mozconfig serve the same purpose?
My thinking is that defining it in the build system *encourages*
checking this kind of code into the tree since it won't affect anyone
else.
- Kyle
Zack Weinberg
> On Tue, Apr 27, 2010 at 4:09 PM, Zack Weinberg
> <zwei...@mozilla.com> wrote:
> >
> > I don't exactly *object*, but ... I use DEBUG_zack to wrap temporary
> > debugging code that I do *not* want to publish. This makes it easy
> > to verify that I've remembered to take it all out again, and if I do
> > push some of it to a shared tree by mistake, it doesn't affect
> > anyone else. I'd like to continue to have something that serves
> > this function.
> >
> > zw
>
> Wouldn't defining DEBUG_zack in your mozconfig serve the same purpose?
Sure, if I had the least idea how to do that.
zw
Robert Strong
export CFLAGS=-DDEBUG_zack
export CXXFLAGS=-DDEBUG_zack
Robert
Zack Weinberg
Does that get copied into the main Makefile, or does it only affect
"make -f client.mk"?
Does that override some other CFLAGS/CXXFLAGS setting that I get by
default?
zw
Nicholas Nethercote
>
> I don't exactly *object*, but ... I use DEBUG_zack to wrap temporary
> debugging code that I do *not* want to publish. This makes it easy to
> verify that I've remembered to take it all out again, and if I do
> push some of it to a shared tree by mistake, it doesn't affect anyone
> else. I'd like to continue to have something that serves this function.
I use comments containing "njn" for this purpose. It doesn't satisfy
the "doesn't affect anyone else" property, but it's really simple.
N
Justin Wood (Callek)
It gets read when you configure (which of course reads the makefile) and
set into the build system...
> Does that override some other CFLAGS/CXXFLAGS setting that I get by
> default?
Nope the rest of the "defaults" stay.
--
~Justin Wood (Callek)
Neil
Although it would look nicer if you could just use ac_add_options
--enable-debug=zack ;-)
--
Warning: May contain traces of nuts.
Benjamin Smedberg
> Although it would look nicer if you could just use ac_add_options
> --enable-debug=zack ;-)
We already allow --enable-debug=-g3
--BDS
Neil
So --enable-debug="-g -DDEBUG_zack" would work?
Ted Mielczarek
> So --enable-debug="-g -DDEBUG_zack" would work?
Yes. The argument just gets inserted into the CFLAGS/CXXFLAGS.
-Ted
Daniel Holbert
automatic "-DDEBUG_<person>" behavior that we've got right now: username
collisions.
A quick MXR search for "#ifdef DEBUG_" comes up with chunks of code
guarded by DEBUG_brendan, DEBUG_dougt, DEBUG_kipp, DEBUG_peter,
DEBUG_vladimir, and DEBUG_warren. (just to pick a few of the most
"common-sounding" names)
It's very likely that some of these OS usernames are shared by multiple
people who compile Mozilla-based projects. As a result of our build
system's auto-define, those people will all get random chunks of debugging
code added to their (debug) builds, probably without their knowledge.
IMHO, we shouldn't automatically impose largely-untested,
infrequently-exercised chunks of debugging code on unsuspecting
developers, simply because they happen to choose an OS username that
collides with one of our committers.