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

don't redundantly call environment_as() to avoid an encoding error for mysterious env var value #6846

Merged
merged 1 commit into from Dec 3, 2018

Conversation

Projects
None yet
3 participants
@cosmicexplorer
Contributor

cosmicexplorer commented Nov 30, 2018

Problem

See https://pantsbuild.slack.com/archives/CB8F24LPM/p1543518771002700 for context -- if that link dies, the issue is: e.g. https://travis-ci.org/pantsbuild/pants/jobs/461402174 on 1.12.x is failing, and this happens to occur in the native toolchain testing. This arises from an incorrectly encoded environment variable which was set somewhere else, I believe in the test harness somewhere. I gleaned that from this excerpt of the build output, where it errors out at trying to set an existing environment variable with environment_as():

pants_test/backend/native/subsystems/test_native_toolchain.py:136: in _invoke_capturing_output
    with environment_as(**env):
/usr/lib/python2.7/contextlib.py:17: in __enter__
    return self.gen.next()
pants/util/contextutil.py:69: in environment_as
    setenv(key, val)
pants/util/contextutil.py:62: in setenv
    os.environ[key] = val if PY3 else _os_encode(val)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

u = '[deferred-sources] fix glob expansion issue in deferred sources mappe\xe2\x80\xa6 (#6824)\n\n### Problem\r\n\r\nWhen ...te target in the second.\r\n\r\nIncludes an adjustment to an integration test that causes it to encounter the failure.'
enc = 'UTF-8'

    def _os_encode(u, enc=sys.getfilesystemencoding()):
      """Turns a `unicode` into `bytes` via encoding."""
>     return u.encode(enc, 'strict')
E     UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 69: ordinal not in range(128)

pants/util/contextutil.py:41: UnicodeDecodeError

This appears to trigger nondeterministically. I wouldn't be surprised if the changes in #6824 ended up triggering this, but the real issue is that (it seems) some pants output is being stuffed into an environment variable, and that it happens to not be encoded correctly somehow. I'm not sure why this is the case, but if it is process output of some form, I might be able to see why this is nondeterministic.

So the underlying issue is twofold -- one that we're stuffing process output into an env var (which might be legitimate), and two that we aren't encoding the value of the env var we're setting correctly. For now, this commit allows us to avoid triggering the underlying issue.

Solution

  • Remove the unnecessary environment_as() call with os.environ.copy() in the native toolchain testing to avoid triggering the environment variable issue described above.

Result

  • Test no longer flakes in the 1.12.x branch.
don't copy over the os environment to avoid an encoding error
this arises from an incorrectly encoded environment variable which was set somewhere else, i believe
in the test harness somewhere.

@cosmicexplorer cosmicexplorer merged commit 006f87f into pantsbuild:master Dec 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

wisechengyi added a commit that referenced this pull request Dec 4, 2018

don't copy over the os environment to avoid an encoding error (#6846)
### Problem

See https://pantsbuild.slack.com/archives/CB8F24LPM/p1543518771002700 for context -- if that link dies, the issue is: e.g. https://travis-ci.org/pantsbuild/pants/jobs/461402174 on `1.12.x` is failing, and this happens to occur in the native toolchain testing. This arises from an incorrectly encoded environment variable which was set somewhere else, I believe in the test harness somewhere. I gleaned that from this excerpt of the build output, where it errors out at trying to set an existing environment variable with `environment_as()`:
```
pants_test/backend/native/subsystems/test_native_toolchain.py:136: in _invoke_capturing_output
    with environment_as(**env):
/usr/lib/python2.7/contextlib.py:17: in __enter__
    return self.gen.next()
pants/util/contextutil.py:69: in environment_as
    setenv(key, val)
pants/util/contextutil.py:62: in setenv
    os.environ[key] = val if PY3 else _os_encode(val)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

u = '[deferred-sources] fix glob expansion issue in deferred sources mappe\xe2\x80\xa6 (#6824)\n\n### Problem\r\n\r\nWhen ...te target in the second.\r\n\r\nIncludes an adjustment to an integration test that causes it to encounter the failure.'
enc = 'UTF-8'

    def _os_encode(u, enc=sys.getfilesystemencoding()):
      """Turns a `unicode` into `bytes` via encoding."""
>     return u.encode(enc, 'strict')
E     UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 69: ordinal not in range(128)

pants/util/contextutil.py:41: UnicodeDecodeError
```

This appears to trigger nondeterministically. I wouldn't be surprised if the changes in #6824 ended up triggering this, but the real issue is that (it seems) some pants output is being stuffed into an environment variable, and that it happens to not be encoded correctly somehow. I'm not sure why this is the case, but if it is process output of some form, I might be able to see why this is nondeterministic.

So the underlying issue is twofold -- one that we're stuffing process output into an env var (which might be legitimate), and two that we aren't encoding the value of the env var we're setting correctly. For now, this commit allows us to avoid triggering the underlying issue.

### Solution

- Remove the unnecessary `environment_as()` call with `os.environ.copy()` in the native toolchain testing to avoid triggering the environment variable issue described above.

### Result

- Test no longer flakes in the `1.12.x` branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment