Last Comment Bug 691781 - Remove checked-in xpidl lexer/parser and generate them explicitly during build
: Remove checked-in xpidl lexer/parser and generate them explicitly during build
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
Mentors:
Depends on:
Blocks: 665409
  Show dependency treegraph
 
Reported: 2011-10-04 08:54 PDT by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2011-11-07 04:19 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1 - Add a check to avoid unpacking an empty array. v1 (1.07 KB, patch)
2011-10-04 11:56 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
Details | Diff | Splinter Review
part 2 - Make sure the lexer gets generated with regen, too. v1 (2.94 KB, patch)
2011-10-04 11:57 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
Details | Diff | Splinter Review
part 3 - Generate IDL lexer and parser as part of the build system. v1 (22.21 KB, patch)
2011-10-04 11:58 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
Details | Diff | Splinter Review
part 1 - Add a check to avoid unpacking an empty array. v2 (2.01 KB, patch)
2011-10-10 15:43 PDT, Bobby Holley (:bholley) (busy with Stylo)
bobbyholley: review+
Details | Diff | Splinter Review
part 3 - Generate IDL lexer and parser as part of the build system. v2 (22.37 KB, patch)
2011-10-10 15:44 PDT, Bobby Holley (:bholley) (busy with Stylo)
bobbyholley: review+
Details | Diff | Splinter Review

Description Bobby Holley (:bholley) (busy with Stylo) 2011-10-04 08:54:52 PDT
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) (busy with Stylo) 2011-10-04 09:28:31 PDT
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) (busy with Stylo) 2011-10-04 11:56:38 PDT
Created attachment 564629 [details] [diff] [review]
part 1 - Add a check to avoid unpacking an empty array. v1

This fixes issue 1.
Comment 3 Bobby Holley (:bholley) (busy with Stylo) 2011-10-04 11:57:05 PDT
Created attachment 564630 [details] [diff] [review]
part 2 - Make sure the lexer gets generated with regen, too. v1

This fixes issue 2.
Comment 4 Bobby Holley (:bholley) (busy with Stylo) 2011-10-04 11:58:25 PDT
Created attachment 564631 [details] [diff] [review]
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-06 08:10:38 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-06 08:14:21 PDT
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) (busy with Stylo) 2011-10-10 15:43:23 PDT
Created attachment 566062 [details] [diff] [review]
part 1 - Add a check to avoid unpacking an empty array. v2

Addressed khuey's comments. Carrying over review.
Comment 8 Bobby Holley (:bholley) (busy with Stylo) 2011-10-10 15:44:13 PDT
Created attachment 566063 [details] [diff] [review]
part 3 - Generate IDL lexer and parser as part of the build system. v2

Addressed kyle's comments. Carrying over review.
Comment 10 Ed Morley [:emorley] 2011-11-06 13:57:56 PST
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 12 Bobby Holley (:bholley) (busy with Stylo) 2011-11-07 03:54:43 PST
(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] 2011-11-07 03:58:35 PST
Bug 700203 hasn't fixed the problem in comment 10 unfortunately.
Comment 14 Bobby Holley (:bholley) (busy with Stylo) 2011-11-07 04:08:19 PST
(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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-11-07 04:13:38 PST
Python is just being a POS.
Comment 16 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-11-07 04:19:13 PST
https://hg.mozilla.org/mozilla-central/rev/86d559a09cb0

Note You need to log in before you can comment on or make changes to this bug.