The default bug view has changed. See this FAQ.

Pymake needs to survive native commands doing sys.exit(0)

RESOLVED FIXED in mozilla10

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

unspecified
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/10388675f775
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla10
Created attachment 572374 [details] [diff] [review]
Patch
Attachment #572374 - Flags: review?(ted.mielczarek)
https://hg.mozilla.org/integration/mozilla-inbound/rev/432d201b7606
https://hg.mozilla.org/mozilla-central/rev/432d201b7606
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 572374 [details] [diff] [review]
Patch

Review of attachment 572374 [details] [diff] [review]:
-----------------------------------------------------------------

::: pymake/process.py
@@ +215,5 @@
>              print >>sys.stderr, e
>              return e.exitcode
>          except:
> +            e = sys.exc_info()[1]
> +            if isinstance(e, SystemExit) and e.code == '0':

This doesn't make sense to me. From the Python docs:
http://docs.python.org/library/exceptions.html#exceptions.SystemExit

Is someone actually calling sys.exit('0')? That really should cause the Python interpreter to exit with an error! We should fix the caller in that case, and just check for e.code == 0 or e.code is None here.

::: tests/pycmd.py
@@ +8,5 @@
>    with open(args[0], 'w') as f:
>      f.write(os.environ[args[1]])
> +
> +def asplode(args):
> +  sys.exit(args[0])

This probably wants to be int(args[0]), no?
Attachment #572374 - Flags: review?(ted.mielczarek) → review-
Created attachment 572434 [details] [diff] [review]
Patch

Aha!
Attachment #572374 - Attachment is obsolete: true
Attachment #572434 - Flags: review?(ted.mielczarek)
Comment on attachment 572434 [details] [diff] [review]
Patch

Review of attachment 572434 [details] [diff] [review]:
-----------------------------------------------------------------

::: pymake/process.py
@@ +215,5 @@
>              print >>sys.stderr, e
>              return e.exitcode
>          except:
> +            e = sys.exc_info()[1]
> +            if isinstance(e, SystemExit) and e.code == 0:

I think according to that link I pasted before, this wants to be "e.code == 0 or e.code is None".
Attachment #572434 - Flags: review?(ted.mielczarek) → review+
You need to log in before you can comment on or make changes to this bug.