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

Seek for other sitecustomize.py to import #422

Conversation

linw1995
Copy link
Member

@linw1995 linw1995 commented Apr 28, 2021

Pull Request Check List

  • A news fragment is added in news/ describing what is new.
  • Test cases added for changed code.

Describe what you have changed in this PR.

Try to use nix to build an isolating environment for PDM development. But pdm can not co-exist with Python packages in nix due to the import mechanism that Python only imports module once by name.
The Python in nix using sitecustomize.py to import packages and PDM also using it to enable PEP582 globally.

Hence, we need to make sure that another sitecustomize.py gets imported if it exists. So PDM can make PEP582 enabling globally, and Python in nix also can import its packages.

@linw1995 linw1995 added the 🐛 bug Something isn't working label Apr 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2021

Codecov Report

Merging #422 (4409b82) into master (bb01e47) will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
+ Coverage   83.36%   83.48%   +0.11%     
==========================================
  Files          68       68              
  Lines        5289     5309      +20     
  Branches      937      944       +7     
==========================================
+ Hits         4409     4432      +23     
+ Misses        609      602       -7     
- Partials      271      275       +4     
Flag Coverage Δ
unittests 83.40% <ø> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pdm/models/candidates.py 85.47% <0.00%> (ø)
pdm/cli/commands/cache.py 81.48% <0.00%> (+0.23%) ⬆️
pdm/models/setup.py 55.73% <0.00%> (+1.58%) ⬆️
pdm/models/repositories.py 71.85% <0.00%> (+2.93%) ⬆️

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 e1c4508...4409b82. Read the comment docs.

@frostming frostming changed the title Ensure another sitecustomize imported if exist Seek for other sitecustomize to import Apr 28, 2021
@frostming frostming changed the title Seek for other sitecustomize to import Seek for other sitecustomize.py to import Apr 28, 2021
os.environ.pop("PEP582_PACKAGES", None)
pythonpath = os.environ.pop("PYTHONPATH", "")
pythonpath = remove_pep582_path_from_pythonpath(pythonpath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could make this simpler for testing, just clean the PYTHONPATH here. No need to keep the old value. Test case can provide its value when running

Copy link
Member Author

Choose a reason for hiding this comment

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

We could not assume the development environment of others not taking PYTHONPATH in usage. Some may use nix to do development. And nix is using PYTHONPATH to import packages that may essential to pdm testings. For example, the pip package of python3.9 for downloading wheel files for testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't know the approach used by nix.

tests/test_integration.py Outdated Show resolved Hide resolved
@linw1995 linw1995 force-pushed the bugfix/ensure_another_sitecustomize_imported_if_exist branch from 9395b8e to 4409b82 Compare May 5, 2021 03:27
@frostming frostming merged commit 7450602 into pdm-project:master May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants