Closed
Bug 984511
Opened 11 years ago
Closed 11 years ago
selfhosted.js not being regenerated after changes to included files
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla31
People
(Reporter: bhackett1024, Assigned: froydnj)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
729 bytes,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
As part of bug 979480 I changed js/src/builtin/TypedObjectConstants.h, which is included by the JS engine's self hosted code. This did not seem to cause the selfhosted.js to be regenerated, leading to test failures and the push required a clobber.
Assignee | ||
Comment 1•11 years ago
|
||
Is the selfhosted.js actually used for anything? It looks like it's simply there for debugging purposes; a quick grep doesn't show up selfhosted.js as actually being used.
Do you mean selfhosted.out.h?
Reporter | ||
Comment 2•11 years ago
|
||
OK, yes, selfhosted.out.h looks like the right file.
Assignee | ||
Comment 3•11 years ago
|
||
TypedObject.js #includes TypedObjectConstants.h, but nothing in the build
system knows about that. Let's fix that.
Attachment #8392387 -
Flags: review?(mh+mozilla)
Comment 4•11 years ago
|
||
Comment on attachment 8392387 [details] [diff] [review]
make selfhosted.out.h explicitly depend on TypedObjectConstants.h
Review of attachment 8392387 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/Makefile.in
@@ +382,5 @@
> selfhosted_out_h_deps := \
> $(selfhosting_srcs) \
> $(srcdir)/js.msg \
> $(srcdir)/builtin/embedjs.py \
> + $(srcdir)/builtin/TypedObjectConstants.h \
Are you sure this is the only header needed here? It seems to me we should "just" get the preprocessor invocations out of embedjs.py, and have those generate proper depfiles. That would solve bug 980793 at the same time. Or have embedjs.py have the preprocessor generate depfiles, but this is trickier because it doesn't know the right flags, which, for instance, there isn't for msvc.
Attachment #8392387 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #4)
> > selfhosted_out_h_deps := \
> > $(selfhosting_srcs) \
> > $(srcdir)/js.msg \
> > $(srcdir)/builtin/embedjs.py \
> > + $(srcdir)/builtin/TypedObjectConstants.h \
>
> Are you sure this is the only header needed here?
[froydnj@cerebro gecko-dev.git]$ grep '#include' js/src/builtin/*.js
js/src/builtin/TypedObject.js:#include "TypedObjectConstants.h"
[froydnj@cerebro gecko-dev.git]$
Yes.
> It seems to me we should
> "just" get the preprocessor invocations out of embedjs.py, and have those
> generate proper depfiles. That would solve bug 980793 at the same time. Or
> have embedjs.py have the preprocessor generate depfiles, but this is
> trickier because it doesn't know the right flags, which, for instance, there
> isn't for msvc.
Both of these are essentially "make the (real) preprocessor generate depfiles", which is a lot of pain for not very much gain IMHO.
Alternatively, we could mostly move things out of embedjs.py and have this whole process use preprocessor.py, which would get us most of the way to proper dependencies at the cost of using a sort-of-but-not-quite C-like preprocessor.
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Nathan Froyd (:froydnj) from comment #5)
> Alternatively, we could mostly move things out of embedjs.py and have this
> whole process use preprocessor.py, which would get us most of the way to
> proper dependencies at the cost of using a sort-of-but-not-quite C-like
> preprocessor.
Scratch that. preprocessor.py doesn't strip comments, which ~doubles the size of the uncompressed selfhsoted.js.
Comment 7•11 years ago
|
||
Comment on attachment 8392387 [details] [diff] [review]
make selfhosted.out.h explicitly depend on TypedObjectConstants.h
Review of attachment 8392387 [details] [diff] [review]:
-----------------------------------------------------------------
I guess this is fine for now, then.
Attachment #8392387 -
Flags: review+
Assignee | ||
Comment 8•11 years ago
|
||
Thanks for relenting. :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/73ca2e36b051
Flags: in-testsuite-
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → nfroyd
Comment 9•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•