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

retrace: Map pyc files back to their source #378

Merged
merged 6 commits into from Mar 7, 2019

Conversation

Projects
None yet
2 participants
@kyleam
Copy link
Contributor

kyleam commented Feb 25, 2019

When pyc files come in, convert them to the non-cached name because this is what can be associated with packages (both virtualenv and system). The approach as of this PR's 4ca6ff4 isn't sufficient because it covers only the virtualenv package case. If the pyc file is part of the system's python distribution, it will pass it back out as an unknown file, and DebTracer won't detect it.

In order to deal with this, we need to do two things to unknown files that are subpaths of the virtualenv: (1) apply the pyc -> py mapping over them and (2) widen the symlink resolution to handle cases where the leading path has a directory symlink (e.g., $VENV/lib/python3.5/encodings -> /usr/lib/python3.5/encodings).

kyleam added some commits Feb 25, 2019

ENH: virtualenv: Resolve symlink directories too
If a symlink file under the virtual environment points outside the
environment, we resolve it and replace the original path in the
unknown files set with the resolved path.  This allows other tracers
to detect that file as part of one of their packages.

But virtual environments can also contained linked directories, such
as

  $VENV/lib/python3.5/collections/ -> /usr/lib/python3.5/collections/

To handle these, unconditionally call realpath() on subpaths of the
virtual environment.
RF: retrace: Return distributions and unknown files
This gives api.retrace() callers to inspect the retrace results.
We'll use this in the next test.
ENH: retrace: Try mapping pyc files back to their source
When pyc files come in, convert them to the non-cached name because
the tracers (or at least VenvTracer and DebTracer) do not map pycache
files to packages.  The main decision is _where_ to do this.  Instead
of doing the pyc -> py mapping in all the tracers, do it once at the
retrace entrypoint.

@kyleam kyleam force-pushed the kyleam:venv-pyc branch from 4ca6ff4 to e993aec Feb 25, 2019

@kyleam kyleam marked this pull request as ready for review Feb 25, 2019

@kyleam

This comment has been minimized.

Copy link
Contributor Author

kyleam commented Feb 25, 2019

In order to deal with this, we need to do two things to unknown files that are subpaths of the virtualenv: (1) apply the pyc -> py mapping over them and (2) widen the symlink resolution to handle cases where the leading path has a directory symlink (e.g., $VENV/lib/python3.5/encodings -> /usr/lib/python3.5/encodings).

Actually doing that wouldn't be sufficient because the DebTracer wouldn't know how to handle the pyc files that get passed back to it. So instead of doing no. 1 in all tracers, I've done it once in retrace.py.

@kyleam kyleam changed the title virtualenv: Map pyc files back to their source retrace: Map pyc files back to their source Feb 25, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 25, 2019

Codecov Report

Merging #378 into master will increase coverage by 0.02%.
The diff coverage is 96.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   89.57%   89.59%   +0.02%     
==========================================
  Files         139      139              
  Lines       10277    10307      +30     
==========================================
+ Hits         9206     9235      +29     
- Misses       1071     1072       +1
Impacted Files Coverage Δ
reproman/utils.py 86.03% <100%> (+0.29%) ⬆️
reproman/distributions/venv.py 89.06% <100%> (ø) ⬆️
reproman/tests/test_utils.py 92.72% <100%> (+0.05%) ⬆️
reproman/interface/retrace.py 95.28% <100%> (+0.13%) ⬆️
reproman/distributions/tests/test_venv.py 97.72% <91.66%> (-0.96%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62ca24c...e993aec. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 25, 2019

Codecov Report

Merging #378 into master will increase coverage by 0.09%.
The diff coverage is 96.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   89.57%   89.67%   +0.09%     
==========================================
  Files         139      139              
  Lines       10277    10379     +102     
==========================================
+ Hits         9206     9307     +101     
- Misses       1071     1072       +1
Impacted Files Coverage Δ
reproman/utils.py 86.03% <100%> (+0.29%) ⬆️
reproman/distributions/venv.py 89.06% <100%> (ø) ⬆️
reproman/tests/test_utils.py 92.72% <100%> (+0.05%) ⬆️
reproman/interface/retrace.py 95.28% <100%> (+0.13%) ⬆️
reproman/distributions/tests/test_venv.py 97.72% <91.66%> (-0.96%) ⬇️
reproman/tests/test_installed.py 85% <0%> (-3.24%) ⬇️
reproman/resource/ssh.py 85.71% <0%> (ø) ⬆️
reproman/cmdline/main.py 85.84% <0%> (ø) ⬆️
reproman/formats/__init__.py 100% <0%> (ø) ⬆️
reproman/resource/tests/test_base.py 100% <0%> (ø) ⬆️
... and 113 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62ca24c...61c81e1. Read the comment docs.

Show resolved Hide resolved reproman/utils.py Outdated

@kyleam kyleam added the WiP label Mar 4, 2019

kyleam added some commits Mar 5, 2019

ENH: pycache_source: Consider pyo files too
Update pycache_source() to handle files created under 'python -O ...'.

Re: #378 (review)
TST: Improve readability of pycache_source test
The IDs of the parametrize test cases are in a separate list from the
values.  Given the number of test cases, these IDs aren't very helpful
as descriptions to a reader because it is hard to match an ID to a
test case.  Store the IDs alongside the test values to make this
clearer.

@kyleam kyleam removed the WiP label Mar 5, 2019

@kyleam

This comment has been minimized.

Copy link
Contributor Author

kyleam commented Mar 7, 2019

I'll wait until tomorrow for additional feedback and then merge.

@yarikoptic

This comment has been minimized.

Copy link
Member

yarikoptic commented Mar 7, 2019

Nope, you will not! I will :-)

@yarikoptic yarikoptic merged commit 17af512 into ReproNim:master Mar 7, 2019

3 checks passed

codecov/patch 96.77% of diff hit (target 89.57%)
Details
codecov/project 89.67% (+0.09%) compared to 62ca24c
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kyleam kyleam deleted the kyleam:venv-pyc branch Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.