Random build system cleanup

RESOLVED FIXED in mozilla26

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

(Blocks 1 bug)

Trunk
mozilla26
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch Patch v1Splinter Review
Mostly never-read or never-set variables. Don't hesitate to ask if there's anything non-obvious :)
Attachment #791801 - Flags: review?(mshal)
Comment on attachment 791801 [details] [diff] [review]
Patch v1

>diff --git a/config/rules.mk b/config/rules.mk
>--- a/config/rules.mk
>+++ b/config/rules.mk
>@@ -401,21 +401,16 @@
> 	$(SHARED_LIBRARY:$(DLL_SUFFIX)=.exp) $(wildcard *.ilk) \
> 	$(PROGRAM:$(BIN_SUFFIX)=.exp) $(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.exp) \
> 	$(PROGRAM:$(BIN_SUFFIX)=.lib) $(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.lib) \
> 	$(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.$(OBJ_SUFFIX)) \
> 	$(wildcard gts_tmp_*) $(LIBRARY:%.a=.%.timestamp)
> ALL_TRASH_DIRS = \
> 	$(GARBAGE_DIRS) /no-such-file
> 
>-ifdef QTDIR
>-GARBAGE                 += $(MOCSRCS)
>-GARBAGE                 += $(RCCSRCS)
>-endif
>-

This looks like something I broke during the CPP_SOURCES transition. I didn't realize MOCSRCS was used here - I thought it was a local variable in the Makefiles, and rolled it into CPP_SOURCES. I filed bug 907761 for this. For now it probably makes sense just to leave this out of the patch.

As for RCCSRCS, it doesn't looke like that was ever used, but from bug 703434 it sounds like it might be for some external Qt tools?

>diff --git a/widget/android/Makefile.in b/widget/android/Makefile.in
>--- a/widget/android/Makefile.in
>+++ b/widget/android/Makefile.in
>@@ -7,28 +7,16 @@
> srcdir          = @srcdir@
> VPATH           = @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> LIBRARY_NAME    = widget_android
> EXPORT_LIBRARY  = 1
> LIBXUL_LIBRARY  = 1
>-ifdef MOZ_WEBSMS_BACKEND
>-DEFINES += -DMOZ_WEBSMS_BACKEND
>-endif

Are you sure MOZ_WEBSMS_BACKEND can go away? It is still in configure.in, and the define is used by some sources in widget/android.

Everything else looks great! I'll r+ pending on MOZ_WEBSMS_BACKEND.
Assignee

Comment 2

6 years ago
(In reply to Michael Shal [:mshal] from comment #1)
> Comment on attachment 791801 [details] [diff] [review]
> Patch v1
> 
> >diff --git a/config/rules.mk b/config/rules.mk
> >--- a/config/rules.mk
> >+++ b/config/rules.mk
> >@@ -401,21 +401,16 @@
> > 	$(SHARED_LIBRARY:$(DLL_SUFFIX)=.exp) $(wildcard *.ilk) \
> > 	$(PROGRAM:$(BIN_SUFFIX)=.exp) $(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.exp) \
> > 	$(PROGRAM:$(BIN_SUFFIX)=.lib) $(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.lib) \
> > 	$(SIMPLE_PROGRAMS:$(BIN_SUFFIX)=.$(OBJ_SUFFIX)) \
> > 	$(wildcard gts_tmp_*) $(LIBRARY:%.a=.%.timestamp)
> > ALL_TRASH_DIRS = \
> > 	$(GARBAGE_DIRS) /no-such-file
> > 
> >-ifdef QTDIR
> >-GARBAGE                 += $(MOCSRCS)
> >-GARBAGE                 += $(RCCSRCS)
> >-endif
> >-
> 
> This looks like something I broke during the CPP_SOURCES transition. I
> didn't realize MOCSRCS was used here - I thought it was a local variable in
> the Makefiles, and rolled it into CPP_SOURCES. I filed bug 907761 for this.
> For now it probably makes sense just to leave this out of the patch.

OK.

> As for RCCSRCS, it doesn't looke like that was ever used, but from bug
> 703434 it sounds like it might be for some external Qt tools?

I'm fine either way.

> >diff --git a/widget/android/Makefile.in b/widget/android/Makefile.in
> >--- a/widget/android/Makefile.in
> >+++ b/widget/android/Makefile.in
> >@@ -7,28 +7,16 @@
> > srcdir          = @srcdir@
> > VPATH           = @srcdir@
> > 
> > include $(DEPTH)/config/autoconf.mk
> > 
> > LIBRARY_NAME    = widget_android
> > EXPORT_LIBRARY  = 1
> > LIBXUL_LIBRARY  = 1
> >-ifdef MOZ_WEBSMS_BACKEND
> >-DEFINES += -DMOZ_WEBSMS_BACKEND
> >-endif
> 
> Are you sure MOZ_WEBSMS_BACKEND can go away? It is still in configure.in,
> and the define is used by some sources in widget/android.

Is the AC_DEFINE in configure.in not sufficient?
Comment on attachment 791801 [details] [diff] [review]
Patch v1

r+ assuming MOCSRCS is taken out of this patch.
Attachment #791801 - Flags: review?(mshal) → review+
Assignee

Comment 4

6 years ago
https://hg.mozilla.org/mozilla-central/rev/2a4e30e83513
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.