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

The PEX_TOOLS venv --compile option should ignore compile errors. #2001

Closed
jsirois opened this issue Dec 1, 2022 · 5 comments · Fixed by #2002
Closed

The PEX_TOOLS venv --compile option should ignore compile errors. #2001

jsirois opened this issue Dec 1, 2022 · 5 comments · Fixed by #2002
Labels

Comments

@jsirois
Copy link
Member

jsirois commented Dec 1, 2022

If there is a compile error, it may be irrelevant as in this case https://github.com/ShantanuKumar/pants-multi-poetry.

There the PEX produced via ./pants package src/package-a/package_a:pex_package_a runs into this issue:

$ PEX_TOOLS=1 python3.9 dist/src.package-a.package_a/pex_package_a.pex venv --scope=deps --compile ./pex-deps-compiled 2>&1 | grep -C5 SyntaxError
Compiling './pex-deps-compiled/lib/python3.9/site-packages/aenum/__init__.py'...
Compiling './pex-deps-compiled/lib/python3.9/site-packages/aenum/_py2.py'...
***   File "./pex-deps-compiled/lib/python3.9/site-packages/aenum/_py2.py", line 5
    raise exc, None, tb
             ^
SyntaxError: invalid syntax

Compiling './pex-deps-compiled/lib/python3.9/site-packages/aenum/_py3.py'...
Listing './pex-deps-compiled/lib/python3.9/site-packages/aenum/doc'...
Compiling './pex-deps-compiled/lib/python3.9/site-packages/aenum/test.py'...
Compiling './pex-deps-compiled/lib/python3.9/site-packages/aenum/test_v3.py'...

The aenum/_py2.py file will never get imported by the Python 3.9 interpreter; so this compile failure is not relevant.

If the compile error is real, the application will encounter it at runtime and fail anyhow. On balance, it seems a later warning is by far the lesser of two evils here. Currently failing the venv creation due to failing compilation of a file stops things in their tracks with no recourse.

@jsirois jsirois added the bug label Dec 1, 2022
@jsirois
Copy link
Member Author

jsirois commented Dec 1, 2022

@ShantanuKumar
Copy link
Contributor

Should we then just ignore this error using try/except?

@jsirois
Copy link
Member Author

jsirois commented Dec 1, 2022

That should do. Note that the exception raised is pex.executor.Executor.NonZeroExit:
https://github.com/pantsbuild/pex/blob/61c150d3635b3603ee9a0e8cf9bc748c415a8660/pex/executor.py#L40-L49

That has fields for stdout and stderr; so you could use one or both of those to relay compilation errors as a warning in case the errors are real and will actually blow up when the PEX is run. I'm not sure about how to handle that, being too noisy for some cases and potentailly too quiet for others, but that's where the data is at any rate.

@jsirois
Copy link
Member Author

jsirois commented Dec 1, 2022

If you do go the warning route, you'd use pex.pex_warnings.warn.

@ShantanuKumar
Copy link
Contributor

Hey @jsirois I have finally added an integration test , please have a look.

@jsirois jsirois mentioned this issue Dec 14, 2022
2 tasks
jsirois pushed a commit that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants