890744 - Bad WebIDL dependencies require a clobber

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

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

[Benjamin Smedberg]

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?

Comment 2

[Benjamin Smedberg]

<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?

Comment 3

[Benjamin Smedberg]

Joey can you take this?

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Was this bustage windows-only?

Yes.
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=00c70533490e

Comment 5

[Gregory Szorc [:gps]]

Windows-only could also be explained by pymake bugs. There are known issues like bug 751076 that can cause improper dependency calculation.

Comment 7

[Gregory Szorc [:gps]]

Bug 874923 also deals with WebIDL dependency foo.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Joey, is there an ETA here?  This is being a significant problem causing lots of busted builds and requiring a lot of clobbering.  :(

Comment 9

[Gregory Szorc [:gps]]

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

[Joey Armstrong [:joey]]

(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.

Comment 11

[Ed Morley [:emorley]]

(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

[Joey Armstrong [:joey]]

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?

Comment 13

[Ed Morley [:emorley]]

(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?

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm not actively working on bug 874923, I'm busy with worker thread stuff and memshrink b2g.

Comment 15

[Ed Morley [:emorley]]

gps's lucky day - please may you take a look at this? :-)

Comment 16

[Gregory Szorc [:gps]]

(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.

Comment 17

[Ed Morley [:emorley]]

Superstar! :-D

Comment 18

[Gregory Szorc [:gps]]

Burrito consumed.

Comment 19

[Gregory Szorc [:gps]]

Attachment: Build log of failure

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.

Comment 20

[Gregory Szorc [:gps]]

Attachment: objdir diff after failed build

I put the objdir under Git version control and this is the |git status| output after the failed build.

Comment 21

[Gregory Szorc [:gps]]

Attachment: Ensure updated WebIDL headers are installed after regen

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.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Gregory, thank you for fixing this!

Comment 23

[Gregory Szorc [:gps]]

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

[Mike Hommey [:glandium]]

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)

Comment 25

[Gregory Szorc [:gps]]

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.

Comment 26

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Can you announce that on dev-platform?

Comment 27

[Gregory Szorc [:gps]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #26)
> Can you announce that on dev-platform?

Done.

Comment 28

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/2ebb897ea9e0

Comment 29

[(no longer active)]

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

[Boris Zbarsky [:bzbarsky]]

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

[(no longer active)]

(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.