Raise an exception when unhandled type received in consume_object

RESOLVED FIXED in mozilla28

Status

Firefox Build System
General
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla28

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 828514 [details] [diff] [review]
Raise an exception when unhandled type received in consume_object

Also, remove now useless assignment into self._backend_files.
Attachment #828514 - Flags: review?(gps)
(Assignee)

Updated

5 years ago
Blocks: 935881

Comment 2

5 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

5 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

5 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)

Comment 5

5 years ago
Created attachment 828984 [details] [diff] [review]
Raise an exception when an emitted object is not acknowledged by the build backend

Something like this?
Attachment #828984 - Flags: review?(gps)
(Assignee)

Updated

5 years ago
Attachment #828514 - Attachment is obsolete: true
Attachment #828514 - Flags: review?(gps)
(Assignee)

Comment 6

5 years ago
Created attachment 829117 [details] [diff] [review]
Raise an exception when an emitted object is not acknowledged by the build backend

Now erroring out for unhandled SandboxWrapped too.
Attachment #829117 - Flags: review?(gps)
(Assignee)

Updated

5 years ago
Attachment #828984 - Attachment is obsolete: true
Attachment #828984 - Flags: review?(gps)

Comment 7

5 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+
https://hg.mozilla.org/mozilla-central/rev/37698b2e2330
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.