293485 - Changing an IDL file does not rebuild IDLs that include it (IDL dependencies)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[James Ross]

Many IDL files include others, but no provision is made for this in the
rebuilding of XPTs and Hs. If an IDL file is changed, and another includes it,
the XPT for the second interface will continue to reference the old UUID of the
first, which then promptly breaks when it is used.

I have a new Perl script that checks this dependency tree, and makes XPTs and Hs
rebuild when anything they use changes.

Comment 1

[James Ross]

Attachment: check-idl.pl, my script to check for dependency changes in IDLs

I'm attaching this first, so people can see just what it does and
agree/disagree with the method it takes. The basic idea is to store a cache of
the mtime of all the files used to make an XPT/H, and only rebuild if there is
a change in at least one item.

There are also some tweaks to rules.mk to call this script and rebuild only for
certain return values.

Comment 2

[James Ross]

Attachment: rules.mk changes

Just for completeness, here is my changes to rules.mk to support the Perl
script.

Comment 3

[[e-mail]]

Attachment: xpidl dependency hack that I'll trial tonight

* Adds -d option to xpidl to generate .pp output
* Tells rules.mk that .idl files generate .pp files
I tried it on xpfe/components/search/public and it seems ok, so...

Comment 4

[[e-mail]]

OK, so it doesn't break my build.
But I suppose the real test is to change some .idl file and see what happens.

Comment 5

[Benjamin Smedberg]

Comment on attachment 183489 [details] [diff] [review]
xpidl dependency hack that I'll trial tonight

I don't think I understand the IDLS bits in rules.mk... why use a wildcard
instead of XPIDLSRCS?

Comment 6

[Benjamin Smedberg]

Comment on attachment 183489 [details] [diff] [review]
xpidl dependency hack that I'll trial tonight

other than IDLS = $(strip $(wildcard *.idl)) this looks great! Why can't we use
IDLS = $(XPIDLSRCS) $(SDK_XPIDLSCRCS) or something like that?

Comment 7

[[e-mail]]

Attachment: improved patch and addressed review comments

Comment 8

[[e-mail]]

biesi pointed out that the .h files don't matter as it's the .cpp files that need to track dependency information. This allows me to trim the patch a little.

Comment 9

[[e-mail]]

Attachment: Trimmed patch

Comment 10

[Mike Shaver (:shaver emeritus)]

Comment on attachment 207081 [details] [diff] [review]
Trimmed patch

sr=shaver

Comment 11

[Mike Shaver (:shaver emeritus)]

Comment on attachment 206658 [details] [diff] [review]
improved patch and addressed review comments

cancelling outdated (I think, though the patch isn't obsoleted) sr request

Comment 12

[[e-mail]]

Fix checked in.

Comment 13

[timeless]

I presume this leaks if someone does: -d foo -d bar :)