691781 - Remove checked-in xpidl lexer/parser and generate them explicitly during build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bobby Holley (:bholley)]

Right now, 'make regenerate-idl-parser' is broken in at least two ways:

1 - It throws a python exception while trying to unpack a zero-length array:

http://hg.mozilla.org/mozilla-central/file/51d989ece4c5/xpcom/idl-parser/header.py#l489

2 - It only generates the parser, not the lexer (I'm not yet sure what's wrong here).


Manually-run build deps like this are always prone to break, since the only time they're exercised is when people (like me) try to change something and need to regenerate the files. It also adds more barriers to hacking on the parser.

The generation is instantaneous on my machine, so I don't think there's any reason not to do this during build.

Comment 1

[Bobby Holley (:bholley)]

Ahah!

So ply only writes out the lexer in 'optimize' mode:

http://hg.mozilla.org/mozilla-central/file/51d989ece4c5/other-licenses/ply/ply/lex.py#l1002

Which means that our check here is backwards:

http://hg.mozilla.org/mozilla-central/file/51d989ece4c5/xpcom/idl-parser/xpidl.py#l1380


So as far as I can tell, we should just always pass optimize=1. The only thing special about 'regen' is that we don't have any input files.

Comment 2

[Bobby Holley (:bholley)]

Attachment: part 1 - Add a check to avoid unpacking an empty array. v1

This fixes issue 1.

Comment 3

[Bobby Holley (:bholley)]

Attachment: part 2 - Make sure the lexer gets generated with regen, too. v1

This fixes issue 2.

Comment 4

[Bobby Holley (:bholley)]

Attachment: part 3 - Generate IDL lexer and parser as part of the build system. v1

Now that we've got lexer/parser generation working again, let's kill the checked-in copies.

Flagging khuey for review on everything.

Comment 5

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

Comment on attachment 564629 [details] [diff] [review]
part 1 - Add a check to avoid unpacking an empty array. v1

Does typelib.py not need a similar change?

Comment 6

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

Comment on attachment 564631 [details] [diff] [review]
part 3 - Generate IDL lexer and parser as part of the build system. v1

Review of attachment 564631 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/idl-parser/Makefile.in
@@ +48,5 @@
> +  $(NULL)
> +
> +include $(topsrcdir)/config/rules.mk
> +
> +# Generate the PYL lexer and parser.

PLY, not PYL.

@@ +53,5 @@
> +export:: $(PARSER_SRCS)
> +	$(PYTHON_PATH) \
> +	  -I$(topsrcdir)/other-licenses/ply \
> +	  -I$(topsrcdir)/xpcom/idl-parser \
> +	  $(topsrcdir)/xpcom/idl-parser/header.py --cachedir=. --regen

This should probably depend on ply as well.

@@ +62,5 @@
> +	  -I$(topsrcdir)/xpcom/idl-parser \
> +	  $(topsrcdir)/xpcom/idl-parser/runtests.py
> +
> +clean clobber realclean clobber_all::
> +	rm -f xpidllex.py{,c} xpidlyacc.py{,c} xpidl_debug

Add this stuff to GARBAGE instead.

Comment 7

[Bobby Holley (:bholley)]

Attachment: part 1 - Add a check to avoid unpacking an empty array. v2

Addressed khuey's comments. Carrying over review.

Comment 8

[Bobby Holley (:bholley)]

Attachment: part 3 - Generate IDL lexer and parser as part of the build system. v2

Addressed kyle's comments. Carrying over review.

Comment 9

[Bobby Holley (:bholley)]

Pushed to mozilla-inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b240ea529d1f
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf0e3297e58c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab0a659233a0

Comment 10

[Ed Morley [:emorley]]

Changeset ab0a659233a0 causes the following error on my local windows pymake build:
http://pastebin.mozilla.org/1375910

- Windows 7, Windows SDK v7.0A, MSVC2010, MozillaBuild 1.6rc
- Built using |python -OO ../inbound/build/pymake/make.py -s -j3 -f ../inbound/client.mk| from objdir
- Mozconfig used: http://pastebin.mozilla.org/1375919

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/b240ea529d1f
https://hg.mozilla.org/mozilla-central/rev/cf0e3297e58c
https://hg.mozilla.org/mozilla-central/rev/ab0a659233a0

even if comment 10 is alarming.

Comment 12

[Bobby Holley (:bholley)]

(In reply to Marco Bonardo [:mak] from comment #11)
> even if comment 10 is alarming.

khuey is on this with bug 700203.

Comment 13

[Ed Morley [:emorley]]

Bug 700203 hasn't fixed the problem in comment 10 unfortunately.

Comment 14

[Bobby Holley (:bholley)]

(In reply to Ed Morley [:edmorley] from comment #13)
> Bug 700203 hasn't fixed the problem in comment 10 unfortunately.

Hm, really? That's strange. I think khuey managed to reproduce it, and I'd imagine he would have made sure it fixed the problem before pushing...

Comment 15

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

Python is just being a POS.

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/86d559a09cb0