Closed Bug 291088 Opened 15 years ago Closed 12 years ago
bloat test files should not be included in optimized builds or release builds
http://lxr.mozilla.org/aviary101branch/source/build/Makefile.in#39 Firefox builds contain extra stuff in the /res/ directory that shouldn't be there. In this case, bloattest
Summary: http://lxr.mozilla.org/aviary101branch/source/build/Makefile.in#39 → bloat test files should not be included in optimized builds or release builds
What if I submitted a patch which didn't package these files if --disable-tests is set? bsmedberg, would that be acceptable?
Does this need sr too?
Attachment #278200 - Flags: review?
Attachment #278200 - Flags: review? → review?(benjamin)
Comment on attachment 278200 [details] [diff] [review] Like this You want "ifdef ENABLE_TESTS", but yes this looks correct. I'd like dbaron to confirm that this is ok
Same patch, new ifdef. Carrying over bsmedberg's earlier r+ and dbaron's sr request.
Attachment #278885 - Flags: superreview? → superreview?(dbaron)
It would be nice if we could ifdef based on NS_BUILD_REFCNT_LOGGING, but that's not defined for makefiles and it's a little work to change. Have you at least checked that all the tinderboxes running leak tests are not building with --disable-tests ?
hmm, good point. I checked fxdbug-linux-tbox, bm-xserve11, and nye and none of them have tests enabled. This patch isn't going work.
dbaron, how does this work for you? As far as I can tell, this is the same check which is run when setting NS_BUILD_REFCNT_LOGGING in nsTraceRefcnt.h and it allows me to avoid resorting to configure hackery.
Assignee: mpgritti → ryanvm
Status: NEW → ASSIGNED
Attachment #295518 - Flags: review?
Attachment #295518 - Flags: review? → review?(dbaron)
The check should be for _ENABLE_LOGREFCNT I guess.
Comment on attachment 295519 [details] [diff] [review] Check for MOZ_DEBUG or _ENABLE_LOGREFCNT I don't see any variables that are defined in makefiles based on --enable-logrefcnt. If you wanted one, you'd need to add a line to autoconf.mk.in to match the currently unused AC_SUBST in configure.in for MOZ_LOG_REFCNT. But it might be more appropriate to just make these check ENABLE_TESTS. They're not really specific to refcount logging -- they're general tests that can be used with any type of logging. (But we'd need to check that the necessary tinderboxes had tests enabled.)
Attachment #295519 - Flags: review?(dbaron) → review-
As pointed out in comment #6, none of the debug tinderboxen have tests enabled AFAICT.
"tests enabled" == "doesn't use --disable-tests" (since tests are on by default)
Attachment #295519 - Attachment is obsolete: true
Comment on attachment 278885 [details] [diff] [review] Proper ifdef action Unobsoleting per comment #9 and comment #11. I've checked, and none of the Tinderbox debug machines have --disable-tests set, so I guess we're in the clear. This already had r+, so asking for approval1.9.
Comment on attachment 278885 [details] [diff] [review] Proper ifdef action a=beltzner for 1.9
Attachment #278885 - Flags: approval1.9? → approval1.9+
Assignee: ryanvm → nobody
Status: ASSIGNED → NEW
Component: Embedding: GTK Widget → Build Config
OS: Windows 2000 → All
QA Contact: pavlov → build-config
Hardware: PC → All
Checking in build/Makefile.in; /cvsroot/mozilla/build/Makefile.in,v <-- Makefile.in new revision: 1.20; previous revision: 1.19 done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
You need to log in before you can comment on or make changes to this bug.