Closed Bug 691781 Opened 8 years ago Closed 8 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla10

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

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.
This fixes issue 1.
Attachment #564629 - Flags: review?(khuey)
This fixes issue 2.
Attachment #564630 - Flags: review?(khuey)
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+
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+
Addressed khuey's comments. Carrying over review.
Attachment #564629 - Attachment is obsolete: true
Attachment #566062 - Flags: review+
Addressed kyle's comments. Carrying over review.
Attachment #564631 - Attachment is obsolete: true
Attachment #566063 - Flags: review+
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
(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...
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.