Last Comment Bug 748001 - make in toolkit/library will make in dependent dirs, but not quite correctly (no LOCAL_INCLUDES?)
: make in toolkit/library will make in dependent dirs, but not quite correctly ...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: 755277 757594
Blocks: 644608
  Show dependency treegraph
 
Reported: 2012-04-23 10:58 PDT by Boris Zbarsky [:bz] (TPAC)
Modified: 2012-05-22 13:26 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use explicit targets for object files (16.46 KB, patch)
2012-05-02 06:26 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (TPAC) 2012-04-23 10:58:37 PDT
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 Ted Mielczarek [:ted.mielczarek] 2012-04-23 11:03:17 PDT
Probably fallout from bug 644608.
Comment 2 Mike Hommey [:glandium] 2012-04-23 11:04:52 PDT
(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] 2012-05-02 06:26:08 PDT
Created attachment 620283 [details] [diff] [review]
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] 2012-05-02 06:27:53 PDT
Forgot to mention, this got an all-green: https://tbpl.mozilla.org/?tree=Try&rev=4623a6b1e9b9
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-05-14 13:50:03 PDT
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] 2012-05-14 22:47:28 PDT
(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] 2012-05-14 22:51:31 PDT
FWIW, leaksoup, leakstats and bloatblame are mentioned on the wiki.
Comment 9 Ed Morley [:emorley] 2012-05-15 06:28:13 PDT
https://hg.mozilla.org/mozilla-central/rev/43cc6c43f79c

Note You need to log in before you can comment on or make changes to this bug.