463824 - Introduce GLOBAL_DEPS to build system

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Justin Wood (:Callek)]

Attachment: Introduce GLOBAL_DEPS

We should introduce GLOBAL_DEPS to our build system, in order to have a consistent method to globally depend on files that should cause our entire source to rebuild.

This patch also introduces autoconf.mk into the global deps.

Should we add $(topsrcdir)/config/config.mk to the list as well?

This blocks SeaMonkey as we need to be able to globally depend on app-config.mk (we use a DEFINE there). Should we globally add app-config.mk (when it exists) or have each app that wants it simply add to GLOBAL_DEPS in app-config.mk itself?

NOTES:
* Did not touch gfx/cairo as it appears that is all from upstream and I'd rather not poke code I'm not too sure the rules on
* Did not touch js/ its my understanding that area of code is still fairly well regulated and even has its own independant config (due to tamarin). If you prefer I fix that up as well, I'll add it into either an iteration of this patch, or a new bug at your choice.
* Comm-central I'll do in its own bug.

Comment 1

[Justin Wood (:Callek)]

ted any ETA on this review, it does block easy landing of a SM2 Blocker

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 347072 [details] [diff] [review]
Introduce GLOBAL_DEPS

+# Global Depends
+GLOBAL_DEPS += Makefile Makefile.in $(DEPTH)/config/autoconf.mk

Could you make this comment slightly more informative?

Patch looks ok to me, sorry for the wait.

Comment 3

[Justin Wood (:Callek)]

Comment on attachment 347072 [details] [diff] [review]
Introduce GLOBAL_DEPS

Probably worth waiting until after b2 to push this...

Ted, any suggestions as to a better comment? (feel free to ping the answer in IRC)

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347072 [details] [diff] [review]
Introduce GLOBAL_DEPS

a191=beltzner

Comment 5

[Justin Wood (:Callek)]

Attachment: Introduce GLOBAL_DEPS [for checkin]

Address comment as best as I can come up with, my c#0 Q's were not answered so I'll just file followup bugs on all them... (they should not block this anyway)

Comment 6

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/87f6ae0c4324

Comment 7

[Shawn Wilsher :sdwilsh]

I had to back this out.  The files in js/config need to be updated as well.
http://hg.mozilla.org/mozilla-central/rev/21d4cb73f6fe (complete with wrong bug number in comment!)

Comment 8

[Justin Wood (:Callek)]

Attachment: Introduce GLOBAL_DEPS [for checkin] [include js/]

Actual patch for checkin, updated js/config/rules.mk did not touch anything else there.

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 350393 [details] [diff] [review]
Introduce GLOBAL_DEPS [for checkin] [include js/]

Ok, I stupidly refreshed with -s (which doesn't look for new changed files)

Which turned tree orange again... so I just pushed the fix:

$ hg outgoing -v
running "ssh hg.mozilla.org "hg -R mozilla-central/ serve --stdio""
comparing with ssh://hg.mozilla.org/mozilla-central/
searching for changes
changeset:   22062:0e1de1e5e5b1
tag:         tip
user:        Justin Wood <[e-mail]>
date:        Thu Nov 27 21:58:44 2008 -0500
files:       js/src/config/rules.mk
description:
Bug 463824, Introduce GLOBAL_DEPS to build system
r=ted, a1.9.1=beltzner [js bits rs=sdwilsh]
(really include js bits this time, forgot to *not* use |hg qref -s|)

$ hg push
pushing to ssh://hg.mozilla.org/mozilla-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files

Comment 10

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/367be4e84186

Comment 11

[Robert Kaiser]

Comment on attachment 350393 [details] [diff] [review]
Introduce GLOBAL_DEPS [for checkin] [include js/]

>diff --git a/config/rules.mk b/config/rules.mk
>+# Dependancies which, if modified, should cause everything to rebuild
>+GLOBAL_DEPS += Makefile Makefile.in $(DEPTH)/config/autoconf.mk

Erm, isn't the word "Dependencies", with an "e" instead of an "a"?

Comment 12

[Justin Wood (:Callek)]

...patch landed before branch, removing wanted?