976885 - Port RCFLAGS to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[(no longer active)]

Attachment: Port RCFLAGS to moz.build

Comment 2

[Mike Hommey [:glandium]]

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?

Comment 3

[Mike Hommey [:glandium]]

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

Comment 4

[Mike Hommey [:glandium]]

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

Comment 5

[(no longer active)]

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

Comment 6

[Mike Hommey [:glandium]]

Yes, and replace the few RCFLAGS += -Dsomething with DEFINES[something] = True

And, obviously, check that this works properly.

Comment 7

[(no longer active)]

Attachment: Port RCFLAGS to moz.build

Comment 8

[Mike Hommey [:glandium]]

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?

Comment 9

[(no longer active)]

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

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/58053e39e2d4

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/58053e39e2d4