xpt.py is trying to generate the destination file even if all inputs files are older (ie. unchanged). We should prevent that to win some time. This is currently not changing anything because the dest file is removed before calling this method but we might and up not removing (maybe only if an env variable is set).

Comment on attachment 578948 [details] [diff] [review] Patch v1 Review of attachment 578948 [details] [diff] [review]: ----------------------------------------------------------------- This should be handled in the build system, not in xpt.py.

The file isn't called from a Makefile but from toolkit/mozapps/installer/xptlink.pl which is called by a Makefile. Adding a rule to the Makefile to handle that might not be trivial. AFAIUI, that would mean translating the perl script to some makefile rules. This said, can a makefile rule be dynamic? In the sense that components can be numerous and the files needed to build them vary too. FWIW, I think that would be quite too much of work compared to the goal we try to reach.

Comment on attachment 578948 [details] [diff] [review] Patch v1 Ok, since all this shit is awful adding this doesn't make it much worse.

I must say I don't quite like this change, because it has an general effect on a tool that is used in more than our trees (it is part of the sdk)

Backed out for win64 make check failures: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=5b2cef6c4c34 https://tbpl.mozilla.org/php/getParsedLog.php?id=11319103&tree=Mozilla-Inbound { make -C tools check make[4]: Entering directory `/e/builds/moz2_slave/m-in-w64/build/obj-firefox/xpcom/typelib/xpt/tools' c:/mozilla-build/python/python2.6.exe /e/builds/moz2_slave/m-in-w64/build/xpcom/typelib/xpt/tools/runtests.py c:\users\cltbld\appdata\local\temp\tmpzwsnlh\tmpt5biin is already up-to-date. make[4]: Leaving directory `/e/builds/moz2_slave/m-in-w64/build/obj-firefox/xpcom/typelib/xpt/tools' make[3]: Leaving directory `/e/builds/moz2_slave/m-in-w64/build/obj-firefox/xpcom/typelib/xpt' make[2]: Leaving directory `/e/builds/moz2_slave/m-in-w64/build/obj-firefox/xpcom/typelib' make[1]: Leaving directory `/e/builds/moz2_slave/m-in-w64/build/obj-firefox/xpcom' ....................E ====================================================================== ERROR: test_xpt_link (__main__.TestXPTLink) ---------------------------------------------------------------------- Traceback (most recent call last): File "e:/builds/moz2_slave/m-in-w64/build/xpcom/typelib/xpt/tools/runtests.py", line 780, in test_xpt_link t3 = xpt.Typelib.read(f3) File "e:\builds\moz2_slave\m-in-w64\build\xpcom\typelib\xpt\tools\xpt.py", line 1063, in read data = Typelib._header.unpack(map[:Typelib._header.size]) error: unpack requires a string argument of length 32 ---------------------------------------------------------------------- Ran 21 tests in 0.020s FAILED (errors=1) make[4]: *** [check] Error 1 } https://hg.mozilla.org/integration/mozilla-inbound/rev/043266d76bb3

Oddly enough, this passed the Win64 build on try: https://tbpl.mozilla.org/php/getParsedLog.php?id=11316618&tree=Try

It's most probably an incremental build issue. I still don't think touching xpt.py is the right way to fix this.

I have a patch on bug 654448 that makes `xpt.py link` about 5x faster, so maybe we don't need this.

(In reply to Ted Mielczarek [:ted] from comment #9) > I have a patch on bug 654448 that makes `xpt.py link` about 5x faster, so > maybe we don't need this. Just for information, does adding this change to your patch has a serious impact in the time time |make package| takes?

I don't know, I haven't tested this patch. My patch in the other bug took the xpt link step from 16.5s to 3.5s on my machine.

mounir: We've got enough speedup from (now closed) bug#707569 that I'm happy for phase1 cleanup, hence fixing dependencies. Plus blocking a fixed bug didnt make sense. Note: I didnt close this bug#707579, because I *think* this is still useful to do, so happy to help make that happen if you think its worth it. I dont have full context, hence asking: what do you want to do here?

Given that Glandium was quite against that patch, I wouldn't mind marking it WONTFIX. Otherwise, we can keep it open if we want to reduce the |make package| time even more...

Bug 707579 can still be fixed, but without altering xpt.py normal behaviour. (It might be possible to implement with "simple" Makefile dependencies)

I'm not sure if this is an issue or not, but it is more of a build system issue than an XPCOM issue.