The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla10

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

unspecified
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

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.
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.
Created attachment 564629 [details] [diff] [review]
part 1 - Add a check to avoid unpacking an empty array. v1

This fixes issue 1.
Attachment #564629 - Flags: review?(khuey)
Created attachment 564630 [details] [diff] [review]
part 2 - Make sure the lexer gets generated with regen, too. v1

This fixes issue 2.
Attachment #564630 - Flags: review?(khuey)
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.
Attachment #564631 - Flags: review?(khuey)
Blocks: 665409
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?
Attachment #564629 - Flags: review?(khuey) → review+
Attachment #564630 - Flags: review?(khuey) → review+
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.
Attachment #564631 - Flags: review?(khuey) → review+
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.
Attachment #564629 - Attachment is obsolete: true
Attachment #566062 - Flags: review+
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.
Attachment #564631 - Attachment is obsolete: true
Attachment #566063 - Flags: review+
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
Target Milestone: --- → mozilla10
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
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.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(In reply to Marco Bonardo [:mak] from comment #11)
> even if comment 10 is alarming.

khuey is on this with bug 700203.
Bug 700203 hasn't fixed the problem in comment 10 unfortunately.
(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...
Python is just being a POS.
https://hg.mozilla.org/mozilla-central/rev/86d559a09cb0
You need to log in before you can comment on or make changes to this bug.