Last Comment Bug 770426 - make::rules.mk - replace double colon rules with dependency chains for Preprocessing rules
: make::rules.mk - replace double colon rules with dependency chains for Prepro...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Mike Hommey [:glandium]
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 776503 927088
Blocks: 668796 767839 773202
  Show dependency treegraph
 
Reported: 2012-07-02 23:59 PDT by Mike Hommey [:glandium]
Modified: 2013-10-28 11:30 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Replace double colon rules with dependency chains for preprocessing rules (11.49 KB, patch)
2012-07-03 01:36 PDT, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Splinter Review
Fixup (4.34 KB, patch)
2012-07-23 06:38 PDT, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Splinter Review
Replace double colon rules with dependency chains for preprocessing rules. (14.30 KB, patch)
2012-07-26 05:44 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Replace double colon rules with dependency chains for preprocessing rules. (15.05 KB, patch)
2012-07-30 10:43 PDT, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-07-02 23:59:25 PDT

    
Comment 1 Mike Hommey [:glandium] 2012-07-03 01:36:06 PDT
Created attachment 638619 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules

This only covers the cases where we preprocess $file to some other location. The cases where we preprocess $srcdir/$file.in to $file are not covered.
Comment 2 Gregory Szorc [:gps] 2012-07-03 09:43:34 PDT
Drive-by review: while you are here, do you want to move functionality into its own .mk file?
Comment 3 Mike Hommey [:glandium] 2012-07-03 09:52:49 PDT
(In reply to Gregory Szorc [:gps] from comment #2)
> Drive-by review: while you are here, do you want to move functionality into
> its own .mk file?

I'd rather leave that to someone who'd have the time to do that and write tests.
Comment 5 Mike Hommey [:glandium] 2012-07-23 06:38:04 PDT
Created attachment 644913 [details] [diff] [review]
Fixup

When landing, we got failures on clobber builds:

Traceback (most recent call last):
  File "e:/builds/moz2_slave/m-in-w32-pgo/build/config/buildlist.py", line 40, in <module>
    addEntriesToListFile(sys.argv[1], sys.argv[2:])
  File "e:/builds/moz2_slave/m-in-w32-pgo/build/config/buildlist.py", line 19, in addEntriesToListFile
    lock = lockFile(listFile + ".lck")
  File "e:\builds\moz2_slave\m-in-w32-pgo\build\config\utils.py", line 44, in lockFile
    fd = os.open(lockfile, os.O_EXCL | os.O_RDWR | os.O_CREAT)
OSError: [Errno 2] No such file or directory: '../dist/bin/webapprt/chrome.manifest.lck'

The reason is subtle: the patch from this bug effectively reorders some rules, unhiding a latent bug in the chome.manifest rules, which the definitions in webapprt/Makefile.in made it work by chance: before, the rule to copy the pref file were appearing before the chrome.manifest rule, so $(FINAL_TARGET) was already created. With the patch from this bug, the rule to copy the pref file is appearing after the chrome.manifest rule, so $(FINAL_TARGET) is not created before running the chrome.manifest rule.

This adds proper dependencies.
Comment 7 Ed Morley [:emorley] 2012-07-25 01:37:44 PDT
Push backed out for Windows mochitest crashes:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=eecd3aa199e6

https://hg.mozilla.org/integration/mozilla-inbound/rev/1abfd50c8be6
Comment 8 Mike Hommey [:glandium] 2012-07-26 05:44:33 PDT
Created attachment 646106 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

This fixes the issue we had on Windows when landing (it turns out the "target" variable was set in the environment, thus overriding whatever was in rules.mk), and makes the rule more similar to those of bug 773202 (btw, if you want to review that one too... ;) ).
Comment 9 Mike Hommey [:glandium] 2012-07-26 08:39:20 PDT
Comment on attachment 646106 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

Still breaks win debug, apparently :(
Comment 10 Mike Hommey [:glandium] 2012-07-30 10:43:22 PDT
Created attachment 647216 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

Finally nailed it.
Comment 11 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-06 07:54:30 PDT
Comment on attachment 647216 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

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

mmm eval
Comment 13 Ed Morley [:emorley] 2012-08-07 07:33:38 PDT
https://hg.mozilla.org/mozilla-central/rev/4d183847fc69
Comment 14 Gregory Szorc [:gps] 2012-08-08 16:29:19 PDT
*** Bug 701393 has been marked as a duplicate of this bug. ***

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