Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use NGContext.exit() instead of sys.exit(). #8239

Merged
merged 1 commit into from Sep 4, 2019

Conversation

olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Sep 4, 2019

Problem

Previously, a compile error would cause the zinc wrapper to call sys.exit(1), which killed the hot nailgun JVM process.

Solution

This commit replaces sys.exit(1) with a call to NGContext.exit(1), which is a Nailgun API to mimic sys.exit(1), allowing the existing JVM process to continue running.

Result

Users should experience a faster compile/debug workflows while iterating on code due to fewer JVM startups.

Problem
----
Previously, a compile error would cause the zinc wrapper to call `sys.exit(1)`, which killed the hot nailgun JVM process.

Solution
----
This commit replaces `sys.exit(1)` with a call to `NGContext.exit(1)`, which is a Nailgun API to mimic `sys.exit(1)`, allowing the existing JVM process to continue running.

Result
----
Users should experience a faster compile/debug workflows while iterating on code due to fewer JVM startups.
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@stuhood stuhood merged commit d4676d1 into pantsbuild:master Sep 4, 2019
stuhood added a commit that referenced this pull request Sep 22, 2019
### Problem

#8239 uses a nailgun API to avoid exiting: we need to incorporate it. Additionally, the zinc-extractor is no longer used after #8125.

### Solution

Remove the `zinc-extractor` code, and bump to latest `zinc-compiler`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants