Comment on attachment 8381827 [details] [diff] [review] Port RCFLAGS to moz.build Review of attachment 8381827 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/app/moz.build @@ +58,5 @@ > + RCFLAGS += ['-DMOZ_PHOENIX'] > + if CONFIG['GNU_CC']: > + RCFLAGS += ['--include-dir ' + SRCDIR] > + else: > + RCFLAGS += ['-I' + SRCDIR] The --include-dir/-I business sounds like it should just be part of the command by default, instead of adding the same snippet over and over. As for the remaining RCFLAGS, which are all -D flags, can't we just use DEFINES?

In fact, the command already uses INCLUDES and DEFINES... (although that filter-out should probably be on both ends) ifdef GNU_CC $(RC) $(RCFLAGS) $(filter-out -U%,$(DEFINES)) $(INCLUDES:-I%=--include-dir %) $(OUTOPTION)$@ $(_VPATH_SRCS) else $(RC) $(RCFLAGS) -r $(DEFINES) $(INCLUDES) $(OUTOPTION)$@ $(_VPATH_SRCS) endif

(and INCLUDES should already have -I$(srcdir) in it)

So are you saying that I should just remove the variable?

Yes, and replace the few RCFLAGS += -Dsomething with DEFINES[something] = True And, obviously, check that this works properly.

Comment on attachment 8381901 [details] [diff] [review] Port RCFLAGS to moz.build Review of attachment 8381901 [details] [diff] [review]: ----------------------------------------------------------------- r+ provided the answer to the question below is yes. ::: webapprt/win/Makefile.in @@ -36,5 @@ > -else > -RCFLAGS += --include-dir $(srcdir) > -endif > -ifdef DEBUG > -RCFLAGS += -DDEBUG Do we really have -DDEBUG in DEFINES on debug builds?

(In reply to Mike Hommey [:glandium] from comment #8) > Comment on attachment 8381901 [details] [diff] [review] > Port RCFLAGS to moz.build > > Review of attachment 8381901 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ provided the answer to the question below is yes. > > ::: webapprt/win/Makefile.in > @@ -36,5 @@ > > -else > > -RCFLAGS += --include-dir $(srcdir) > > -endif > > -ifdef DEBUG > > -RCFLAGS += -DDEBUG > > Do we really have -DDEBUG in DEFINES on debug builds? Hmm, I thought we did, but it seems like we don't. I'll fix this when landing.