748001 - make in toolkit/library will make in dependent dirs, but not quite correctly (no LOCAL_INCLUDES?)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1)  Change content/canvas/src/WebGLContextGL.cpp
2)  Run make -C $objdir/toolkit/library

ACTUAL RESULTS:

  WebGLContextGL.cpp
  In file included from content/canvas/src/WebGLContextGL.cpp:41:
  In file included from content/canvas/src/WebGLContext.h:55:
  ../../dist/include/nsHTMLCanvasElement.h:42:10: fatal error:
    'nsGenericHTMLElement.h' file not found
  #include "nsGenericHTMLElement.h"

EXPECTED RESULTS: in an ideal world, total victory, just making in toolkit/library builds everything correctly.  ;)

Comment 1

[(not currently active) Ted Mielczarek]

Probably fallout from bug 644608.

Comment 2

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted] from comment #1)
> Probably fallout from bug 644608.

Not probably.

(In reply to Boris Zbarsky (:bz) from comment #0)
> STEPS TO REPRODUCE:
> 
> 1)  Change content/canvas/src/WebGLContextGL.cpp
> 2)  Run make -C $objdir/toolkit/library

You're shooting yourself in the foot here ;)

Comment 3

[Mike Hommey [:glandium]]

Attachment: Use explicit targets for object files

In practice, this means removing what makes e.g. setting SIMPLE_PROGRAMS
work without any CPPSRCS set.

Fortunately, only a few Makefiles rely on such constructs.

Comment 4

[Mike Hommey [:glandium]]

Forgot to mention, this got an all-green: https://tbpl.mozilla.org/?tree=Try&rev=4623a6b1e9b9

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 620283 [details] [diff] [review]
Use explicit targets for object files

Review of attachment 620283 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/rules.mk
@@ -1137,5 @@
>  
> -%:: %.c $(GLOBAL_DEPS)
> -	$(REPORT_BUILD)
> -	@$(MAKE_DEPS_AUTO_CC)
> -	$(ELOG) $(CC) $(CFLAGS) $(LDFLAGS) $(OUTOPTION)$@ $(_VPATH_SRCS)

We don't actually rely on this rule anywhere silly, right?

::: tools/trace-malloc/Makefile.in
@@ +59,5 @@
>  
>  SIMPLE_PROGRAMS	= \
>  		$(SIMPLECSRCS:.c=$(BIN_SUFFIX)) \
>  		$(SIMPLECPPSRCS:.cpp=$(BIN_SUFFIX)) \
>  		$(NULL)

I wonder if we even use any of these programs anymore? Maybe we could just remove all this?

Comment 6

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted] from comment #5)
> Comment on attachment 620283 [details] [diff] [review]
> Use explicit targets for object files
> 
> Review of attachment 620283 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: config/rules.mk
> @@ -1137,5 @@
> >  
> > -%:: %.c $(GLOBAL_DEPS)
> > -	$(REPORT_BUILD)
> > -	@$(MAKE_DEPS_AUTO_CC)
> > -	$(ELOG) $(CC) $(CFLAGS) $(LDFLAGS) $(OUTOPTION)$@ $(_VPATH_SRCS)
> 
> We don't actually rely on this rule anywhere silly, right?

AFAIK, we don't. At least not when building Firefox.
 
> ::: tools/trace-malloc/Makefile.in
> @@ +59,5 @@
> >  
> >  SIMPLE_PROGRAMS	= \
> >  		$(SIMPLECSRCS:.c=$(BIN_SUFFIX)) \
> >  		$(SIMPLECPPSRCS:.cpp=$(BIN_SUFFIX)) \
> >  		$(NULL)
> 
> I wonder if we even use any of these programs anymore? Maybe we could just
> remove all this?

Let's possibly do that in a separate bug.

Comment 7

[Mike Hommey [:glandium]]

FWIW, leaksoup, leakstats and bloatblame are mentioned on the wiki.

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43cc6c43f79c

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/43cc6c43f79c