Closed
Bug 890744
Opened 11 years ago
Closed 11 years ago
Bad WebIDL dependencies require a clobber
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla26
People
(Reporter: RyanVM, Assigned: gps)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
Some sort of WebIDL dependency issue? Logs: https://tbpl.mozilla.org/php/getParsedLog.php?id=24995789&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=24995791&tree=Mozilla-Inbound The push in question: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=00c70533490e
Comment 1•11 years ago
|
||
Was this bustage windows-only? Details: * Source commit added RecordErrorEvent.webidl, added RecordErrorEvent.webidl to dom/webidl/WebIDL.mk, added unrelated entries to content/media/moz.build * "Reticulating Splines" is present * We are running the globalgen.py to recreate ParserResults.pkl and RecordErrorEvent.webidl is present in the command line * We are running the commands to recreate .BindingGen According to bz, globalgen.py is responsible for writing the new mozilla::dom::prototypes::id, so there is likely something internal to the python script that is screwing up. Does it do its own internal dependency system?
Flags: needinfo?(ryanvm)
Comment 2•11 years ago
|
||
<bz> I wonder... <bz> 13 EXPORTS.mozilla.dom += [ <bz> 25 'PrototypeList.h', <bz> When would the export happen, compared to the GlobalGen execution? <bz> My best guess is that the file got copied to dist/include <bz> and _after_ that GlobalGen ran <bz> or something <bsmedberg> yeah, that sounds kinda likely <bsmedberg> which would explain the windows-only bit, since other platforms use symlinks I don't see the actual "copy PrototypeList.h to dist/include" bit anywhere in the log... I guess it's a hidden command?
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #1) > Was this bustage windows-only? Yes. https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=00c70533490e
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 5•11 years ago
|
||
Windows-only could also be explained by pymake bugs. There are known issues like bug 751076 that can cause improper dependency calculation.
Assignee | ||
Comment 7•11 years ago
|
||
Bug 874923 also deals with WebIDL dependency foo.
Summary: Bug 803414 required a clobber on Windows → Bad WebIDL dependencies require a clobber
Comment 8•11 years ago
|
||
Joey, is there an ETA here? This is being a significant problem causing lots of busted builds and requiring a lot of clobbering. :(
Flags: needinfo?(joey)
Assignee | ||
Comment 9•11 years ago
|
||
Joey is on holiday until next week Monday IIRC. khuey mentioned this in the hallway the other day. It kinda/sorta sounded like he might be looking into it.
Comment 10•11 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #9) > Joey is on holiday until next week Monday IIRC. khuey mentioned this in the > hallway the other day. It kinda/sorta sounded like he might be looking into > it. I was not looking into this, probably should not have been assigned while I was on vacation.
Flags: needinfo?(joey)
Comment 11•11 years ago
|
||
(In reply to Joey Armstrong [:joey] from comment #10) > I was not looking into this, probably should not have been assigned while I > was on vacation. Ah, that's unfortunate. Will you be able to look into it now, or would you prefer I get gps or someone to take a look short term? (This bug is burning a significant amount of automation time, so needs resolving asap).
Comment 12•11 years ago
|
||
bz, bsmedberg, ed: This bug was assigned to me while I was on PTO. Given other work on my plate, it looks like I wont be able to start on this anytime soon, so unassigning. Not sure who else in BuildConfig can take this - maybe khuey, as he is doing another IDL fix in bug#874923, so already has context?
Updated•11 years ago
|
Assignee: joey → nobody
Comment 13•11 years ago
|
||
(In reply to Joey Armstrong [:joey] from comment #12) > bz, bsmedberg, ed: This bug was assigned to me while I was on PTO. Given > other work on my plate, it looks like I wont be able to start on this > anytime soon, so unassigning. Not sure who else in BuildConfig can take this > - maybe khuey, as he is doing another IDL fix in bug#874923, so already has > context?
Flags: needinfo?(khuey)
I'm not actively working on bug 874923, I'm busy with worker thread stuff and memshrink b2g.
Flags: needinfo?(khuey)
Comment 15•11 years ago
|
||
gps's lucky day - please may you take a look at this? :-)
Flags: needinfo?(gps)
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+1] from comment #15) > gps's lucky day - please may you take a look at this? :-) It's *your* lucky day: I'm at home with access to my preferred Windows development machine! Will hopefully look into this once I get a burrito inside of my belly.
Flags: needinfo?(gps)
Comment 17•11 years ago
|
||
Superstar! :-D
Assignee | ||
Comment 19•11 years ago
|
||
The logs in the initial comment aren't loading. But I was able to reproduce a failure. The log from the subsequent build step is attached. STR: 1) hg up -r 1f030ce15fdb 2) Perform full build 3) Save the objdir somewhere (I just put it under version control) 4) hg up -r 00c70533490e 5) mach build -v dom/bindings Now to grok it.
Assignee | ||
Comment 20•11 years ago
|
||
I put the objdir under Git version control and this is the |git status| output after the failed build.
Assignee | ||
Comment 21•11 years ago
|
||
Burritos fuel patches. As speculated in comment #2, this is a Windows-only bug due to behavior of symlinks and the installed header files. Essentially, the GlobalGen.py-produced .h files were not being installed in dist/include because of missing dependencies. This patch adds explicit dependencies and hooks up header installation into the same target as webidl generation. I would appreciate additional verification from Windows users this fixes things. I was doing some wonky things as part of developing this patch, including having to fix merge conflicts between the old revisions I was testing and tip.
Attachment #788454 -
Flags: review?(mh+mozilla)
Comment 22•11 years ago
|
||
Gregory, thank you for fixing this!
Assignee | ||
Comment 23•11 years ago
|
||
So, this bug is hard to test and verify. While I'm not overly confident the patch fixes things, I'm pretty confident it doesn't regress behavior. I'm tempted to just say "if you think it looks good, let's land it and see what happens."
Comment 24•11 years ago
|
||
Comment on attachment 788454 [details] [diff] [review] Ensure updated WebIDL headers are installed after regen Review of attachment 788454 [details] [diff] [review]: ----------------------------------------------------------------- I don't know if this fixes the bug, but at least it looks correct. ::: dom/bindings/Makefile.in @@ +211,5 @@ > @$(TOUCH) $@ > > # Running GlobalGen.py updates ParserResults.pkl as a side-effect > ParserResults.pkl: $(globalgen_dependencies) > + @echo "Generating global WebIDL files." $(info) @@ +230,5 @@ > # what it really needs to regenerate. > # Finally, touch the .BindingGen file so that we don't have to keep redoing > # all that until something else actually changes. > .BindingGen: $(bindinggen_dependencies) $(binding_dependency_trackers) > + @echo "Generating WebIDL bindings." $(info)
Attachment #788454 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 25•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ebb897ea9e0 Please give WebIDL changes the opportunity to bust the tree now. Unless this patch *regresses* behavior, please don't back it out. Instead, reopen this bug and we'll iterate until we have a proper fix.
Can you announce that on dev-platform?
Assignee | ||
Comment 27•11 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #26) > Can you announce that on dev-platform? Done.
Comment 28•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2ebb897ea9e0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment 29•11 years ago
|
||
FWIW bug 871445 got landed and then bounced because of a bug similar to this. It relanded with a CLOBBER change and stuck.
Comment 30•11 years ago
|
||
Can we get a bug filed on that, with some details on exactly what broke? Clearly the build succeeded, so was the issue incorrect generated .cpp or something else?
Comment 31•11 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #30) > Can we get a bug filed on that, with some details on exactly what broke? > Clearly the build succeeded, so was the issue incorrect generated .cpp or > something else? Filed bug 923545 with as much details as I had.
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•