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

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks: 1 bug)

unspecified
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
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.
Attachment #638619 - Flags: review?(khuey)

Comment 2

5 years ago
Drive-by review: while you are here, do you want to move functionality into its own .mk file?
(Assignee)

Comment 3

5 years ago
(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.
Attachment #638619 - Flags: review?(khuey) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/deb1e2c80f84
Target Milestone: --- → mozilla17

Updated

5 years ago
Depends on: 776503
(Assignee)

Comment 5

5 years ago
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.
Attachment #644913 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Blocks: 773202
Attachment #644913 - Flags: review?(khuey) → review+
(Assignee)

Comment 6

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

Comment 7

5 years ago
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
(Assignee)

Comment 8

5 years ago
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... ;) ).
Attachment #646106 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Attachment #638619 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
Comment on attachment 646106 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

Still breaks win debug, apparently :(
Attachment #646106 - Flags: review?(khuey)
(Assignee)

Comment 10

5 years ago
Created attachment 647216 [details] [diff] [review]
Replace double colon rules with dependency chains for preprocessing rules.

Finally nailed it.
Attachment #647216 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Attachment #646106 - Attachment is obsolete: true
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
Attachment #647216 - Flags: review?(khuey) → review+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d183847fc69
https://hg.mozilla.org/mozilla-central/rev/4d183847fc69
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Duplicate of this bug: 701393

Comment 15

4 years ago
http://hg.mozilla.org/comm-central/rev/4cd118a58040
http://hg.mozilla.org/comm-central/rev/2e8458f26a91
(Assignee)

Updated

4 years ago
Depends on: 927088
You need to log in before you can comment on or make changes to this bug.