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

venv create no longer includes --sources-directory contents when all files are nested #2285

Closed
huonw opened this issue Nov 9, 2023 · 1 comment · Fixed by #2286
Closed
Assignees

Comments

@huonw
Copy link
Collaborator

huonw commented Nov 9, 2023

It seems like there was a regression from 2.1.148 -> 2.1.149 with the behaviour of venv create with a --pex-repository that was created with --sources-directory: those sources aren't included in the final venv.

Reproducer:

cd $(mktemp -d)

# create our dummy file
mkdir -p source_files/foo
touch source_files/foo/bar.py # NB.1
# touch source_files/qux.py # NB.2

for version in v2.1.148 v2.1.149; do
    curl -s -L https://github.com/pantsbuild/pex/releases/download/$version/pex > pex-$version
    chmod +x pex-$version

    # NB.3
    ./pex-$version --output-file=repository-$version.pex --sources-directory=source_files

    # NB.4
    PEX_SCRIPT=pex3 ./pex-$version venv create --dest-dir=dest-$version --pex-repository=repository-$version.pex --layout=flat

    # what was included?
    tree dest-$version
done

Running that shows that the contents of the dest-... directory depends on the version, without the bar.py file when using v2.1.149, but should be the same:

dest-v2.1.148
└── foo
    └── bar.py

1 directory, 1 file
dest-v2.1.149

0 directories, 0 files

Ablative studies:

  • uncommenting NB.2 line (to have two files) passes ✅ (both versions have both foo/bar.py and qux.py)
  • replacing the NB.1 with NB.2 (to just qux.py at the top level) passes ✅
  • always using v2.1.148 on line NB.3 (create the pex) and v2.1.149 on line NB.4 (create the venv) passes ✅
  • v2.1.149 for NB.3 and v2.1.148 for NB.4 fails ❌
  • I think third-party dependencies work okay, but haven't confirmed in this reduced setting
  • This reproduces without --layout, but the output is simpler with --layout=flat

(First observed in pantsbuild/pants#20149.)

@huonw
Copy link
Collaborator Author

huonw commented Nov 9, 2023

Bisecting narrows it down to f9a9d94

mkdir -p source_files/foo
touch source_files/foo/bar.py

python -m pex --output-file=repository.pex --sources-directory=source_files
python -m pex.cli venv create --dest-dir=dest --pex-repository=repository.pex --layout=flat
tree dest

@huonw huonw self-assigned this Nov 9, 2023
huonw added a commit that referenced this issue Nov 10, 2023
This fixes #2285 by fixing a minor typo in
f9a9d94 / #2263 that caused the
computation of a PEXes code hash to not recur into the correct
directories.

I think this explains the symptoms in #2285 due to caching: if a PEX
only contains files in subdirectories, the code hash will be
`da39a3ee5e6b4b0d3255bfef95601890afd80709` (the sha256 hash of no data)
and thus potentially the `venv create` might be referring to an
incorrectly cached installation.
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 a pull request may close this issue.

1 participant