291088 - bloat test files should not be included in optimized builds or release builds

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[alanjstr]

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

Comment 1

[Ryan VanderMeulen [:RyanVM]]

What if I submitted a patch which didn't package these files if --disable-tests is set? bsmedberg, would that be acceptable?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Attachment: Like this

Does this need sr too?

Comment 3

[Benjamin Smedberg]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Attachment: Proper ifdef action

Same patch, new ifdef. Carrying over bsmedberg's earlier r+ and dbaron's sr request.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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 ?

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Attachment: Check for MOZ_DEBUG or FORCE_BUILD_REFCNT_LOGGING

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.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: Check for MOZ_DEBUG or _ENABLE_LOGREFCNT

The check should be for _ENABLE_LOGREFCNT I guess.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

As pointed out in comment #6, none of the debug tinderboxen have tests enabled AFAICT.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

"tests enabled" == "doesn't use --disable-tests" (since tests are on by default)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 278885 [details] [diff] [review]
Proper ifdef action

a=beltzner for 1.9

Comment 14

[Reed Loden [:reed]]

Checking in build/Makefile.in;
/cvsroot/mozilla/build/Makefile.in,v  <--  Makefile.in
new revision: 1.20; previous revision: 1.19
done