481350 - Solve the imacros.c.out out of date problem

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brendan Eich [:brendan]]

+++ This bug was initially created as a clone of Bug #476181 +++

Splitting since a patch landed for bug 476181.

/be

Comment 1

[Jason Orendorff [:jorendorff]]

Well, the easy way to hack this is to force a complete two-phase build if you want the JIT.  That can be done with a few lines, I think.  Builds would take twice as long though.

Or configure could let you specify a known-good js shell executable to use for building imacros.c.out.  Or both.

Comment 2

[Benjamin Smedberg]

On IRC we discussed:

* after building, generate an imacros-gen.c.out (in the objdir)
* compare that with the source imacros.c.out. If it doesn't match, the build fails immediately. The file is not automatically copied back to the srcdir, but there is a helpful error message which tells you what to do.

This avoids unintentionally contaminating the srcdir, avoids cleaning up the srcdir version of imacros.c.out in srcdir builds. It requires an explicit step by hackers who change imacros, but I don't think this is a big problem.

Comment 3

[Jason Orendorff [:jorendorff]]

Changes to jsopcode.tbl will also affect imacros.c.out, but I still don't think it's a big problem.

Comment 4

[Benjamin Smedberg]

Attachment: Fail when imacros.c.out is out of date, rev. 1

Comment 5

[Benjamin Smedberg]

Attachment: Fail when imacros.c.out is out of date, rev. 1.1

Forgot a $(wildcard)

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 365972 [details] [diff] [review]
Fail when imacros.c.out is out of date, rev. 1.1

That looks right to me, and it runs here.

Comment 7

[Brendan Eich [:brendan]]

Complete two-phase build is bug 469171, FYI.

Benjamin, thanks for taking this bug.

/be

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 365972 [details] [diff] [review]
Fail when imacros.c.out is out of date, rev. 1.1

Nit: would prefer a more imacros.c.out-related name than imacros-generated.c. Tradition with yacc and lex might use some crazy .yy.tmp suffix, but drop the yy and the relation isn't too ugly:

imacros.c.tmp => imacros.c.out

Suggesting the temporary nature seems winning.

/be

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 365972 [details] [diff] [review]
Fail when imacros.c.out is out of date, rev. 1.1

+ifneq (,$(wildcard $(RUN_TEST_PROGRAM))$(if $(NSPR_LIBS),,1))

This is pretty terrible makefile line noise, although I guess your comment does explain it.

+	$(wildcard $(RUN_TEST_PROGRAM)) $(DIST)/bin/js$(BIN_SUFFIX) imacro_asm.js $(srcdir)/imacros.jsasm > $@

Should probably use at least $< here. If you swap the order of the prereqs you could just use $^, no? Similarly in the other rules below, I'd rather have $< where possible.

Also, should fix brendan's naming nit.

Comment 10

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/dc688b5346a6
and bustage followup
http://hg.mozilla.org/mozilla-central/rev/ea23ed5288e6