770426 - make::rules.mk - replace double colon rules with dependency chains for Preprocessing rules

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: 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]]

Drive-by review: while you are here, do you want to move functionality into its own .mk file?

Comment 3

[Mike Hommey [:glandium]]

(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 4

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/deb1e2c80f84

Comment 5

[Mike Hommey [:glandium]]

Attachment: 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 6

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31de6ffdedcb

Comment 7

[Ed Morley [:emorley]]

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]]

Attachment: 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]]

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]]

Attachment: Replace double colon rules with dependency chains for preprocessing rules.

Finally nailed it.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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 12

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d183847fc69

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4d183847fc69

Comment 15

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/4cd118a58040
http://hg.mozilla.org/comm-central/rev/2e8458f26a91