770702 - pymake native nsinstall should sys.exit and not return

Status: RESOLVED INVALID

(Firefox Build System :: General, defect)

Description

[Rain [:rain1] (inactive)]

Attachment: patch

Per IRC discussion, pymake eats up return values for native commands.

Comment 1

[Gregory Szorc [:gps]]

Comment on attachment 638885 [details] [diff] [review]
patch

I'm not a build peer, so this isn't an official r+.

The change is trivial, it appears. Just as long as nsinstall() isn't being called from "not pymake" this should be good.

Comment 2

[(not currently active) Ted Mielczarek]

We could also fix pymake.

Comment 3

[Gregory Szorc [:gps]]

(In reply to Ted Mielczarek [:ted] from comment #2)
> We could also fix pymake.

We discussed this heavily on IRC.

Basically, what do you change it to? You can't accept everything because you need a way to convert that to a status code. Is None success or failure? What if you return an object?

I think we decided that a compromise of converting a returned int into a status code is acceptable. If it is anything else, we preserve the existing behavior of assuming success.

Comment 4

[(not currently active) Ted Mielczarek]

Accepting integer return values as status codes sounds pretty sensible. We could probably try to handle other return types, but that doesn't seem really worthwhile.

Comment 5

[Rain [:rain1] (inactive)]

Comment on attachment 638885 [details] [diff] [review]
patch

We realized on IRC that this patch is wrong anyway.

Comment 6

[Rain [:rain1] (inactive)]

I think we're going to fix bug 772186 instead.