Closed
Bug 935870
Opened 11 years ago
Closed 11 years ago
Raise an exception when unhandled type received in consume_object
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla28
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Also, remove now useless assignment into self._backend_files.
Attachment #828514 -
Flags: review?(gps)
Comment 2•11 years ago
|
||
Comment on attachment 828514 [details] [diff] [review]
Raise an exception when unhandled type received in consume_object
Review of attachment 828514 [details] [diff] [review]:
-----------------------------------------------------------------
Can you explain why you want to do this?
If the backend isn't consuming a particular type, I think that will be pretty obvious because the build will fail to be correct. It will also highlight lapses in test coverage.
I'm receptive to putting this in. I just want some additional context.
Attachment #828514 -
Flags: review?(gps) → review-
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 828514 [details] [diff] [review]
Raise an exception when unhandled type received in consume_object
Each time I've added a new object time to be consumed, I was stunned that it wouldn't complain when i ran config.status before adding the handling part in backend_files.
Attachment #828514 -
Flags: review- → review?(gps)
Comment 4•11 years ago
|
||
Here's something else to consider.
It's likely that "special" build components like XPIDL and WebIDL will forever share some consumer/backend functionality. We have code in mozbuild.backend.common that already kinda/sorta takes this approach. A few times now I've started down the road of trying to inject super() into the mix so backends can be composed of multiple classes, each handling a very specific part of the build (like WebIDL). Even my patches in bug 928195 have all WebIDL object consuming happening in a separate class. This patch would break the recursive make backend because that backend explicitly chooses to defer processing of WebIDL classes to another class.
I sympathize with the argument you are making though. Perhaps we should mark consumed objects somehow and enforce (from the emitter) that all objects sent "downstream" come back with a "yes I acked this object" bit set.
Assignee | ||
Updated•11 years ago
|
Attachment #828514 -
Attachment is obsolete: true
Attachment #828514 -
Flags: review?(gps)
Assignee | ||
Comment 6•11 years ago
|
||
Now erroring out for unhandled SandboxWrapped too.
Attachment #829117 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #828984 -
Attachment is obsolete: true
Attachment #828984 -
Flags: review?(gps)
Comment 7•11 years ago
|
||
Comment on attachment 829117 [details] [diff] [review]
Raise an exception when an emitted object is not acknowledged by the build backend
Review of attachment 829117 [details] [diff] [review]:
-----------------------------------------------------------------
Exactly what I was thinking!
Attachment #829117 -
Flags: review?(gps) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Comment 9•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
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
•