Last Comment Bug 680534 - Clean up IDL Directory Rules in rules.mk
: Clean up IDL Directory Rules in rules.mk
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Gregory Szorc [:gps]
:
Mentors:
Depends on:
Blocks: 668796 680369
  Show dependency treegraph
 
Reported: 2011-08-19 12:48 PDT by Gregory Szorc [:gps]
Modified: 2011-08-23 01:27 PDT (History)
3 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove double colon and redundancy of $(IDL_DIR), v1 (1.34 KB, patch)
2011-08-19 12:48 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
Remove double colon and redundancy of $(IDL_DIR), v2 (2.57 KB, patch)
2011-08-19 13:10 PDT, Gregory Szorc [:gps]
khuey: review+
Details | Diff | Splinter Review
Remove double colon and redundancy of $(IDL_DIR), v3 (2.89 KB, patch)
2011-08-22 10:30 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2011-08-19 12:48:31 PDT
Created attachment 554512 [details] [diff] [review]
Remove double colon and redundancy of $(IDL_DIR), v1

Currently, the $(IDL_DIR) target in rules.mk has the following issues:

1) It is defined as a double colon rule, so it gets executed every single time
2) It is defined twice

The attached patch converts it to a regular rule and removes an inner definition. I believe the remaining definition is always in an outer scope and is thus always defined to the bits that need it. Although, the conditional statements in this part of rules.mk are nested pretty deep, so I could be wrong.

I also believe everything writing into $(IDL_DIR) has $(IDL_DIR) as a prerequisite, so removing the double colon should be safe.

I've done the following to verify the change:

* Built w/ make from empty objdir
* Rebuilt a subcomponent w/ make that used to always nsinstall $(IDL_DIR) and verified it didn't do this any more

I'm not sure how much this improves build times by, but it should be greater than 0.
Comment 1 Gregory Szorc [:gps] 2011-08-19 13:10:55 PDT
Created attachment 554520 [details] [diff] [review]
Remove double colon and redundancy of $(IDL_DIR), v2

Added missing identical change to JS's rules.mk and added bug number to summary.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-22 08:41:33 PDT
Comment on attachment 554520 [details] [diff] [review]
Remove double colon and redundancy of $(IDL_DIR), v2

Can we drop the comment above the $(IDL_DIR) rule we're keeping.  I don't think that's been accurate in the better part of a decade ...
Comment 3 Gregory Szorc [:gps] 2011-08-22 10:30:57 PDT
Created attachment 554901 [details] [diff] [review]
Remove double colon and redundancy of $(IDL_DIR), v3

This is the same patch that was r+ by khuey except the outdated comment has been removed, per comment from khuey.
Comment 5 Mounir Lamouri (:mounir) 2011-08-23 01:27:40 PDT
http://hg.mozilla.org/mozilla-central/rev/1ee70d6bb2c8

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