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

pip download: make sure that --use-pep517 is propagated to the dependencies #11022

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Apr 10, 2022

Fixes #9523

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

This makes the behaviour consistent across download, install, and wheel.

@SpecLad
Copy link
Contributor Author

SpecLad commented Apr 17, 2022

I realized that I could make the test run without network access, so I did that.

tests/lib/__init__.py Outdated Show resolved Hide resolved
@SpecLad SpecLad force-pushed the download-propagate-pep517 branch 2 times, most recently from 1535e42 to 539a394 Compare May 1, 2022 18:42
@SpecLad
Copy link
Contributor Author

SpecLad commented May 1, 2022

My previous approach to testing stopped working because of #10717, so I changed the test to use a different approach.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Jun 12, 2022
The approach it uses now doesn't work anymore due to 452d7da.
The installation of `fake_dep` now succeeds whether or not `setuptools`
is installed in the test environment.

Use a different approach instead: try to import `pip` in the `setup.py`
script. If it succeeds, then we are not running in an isolated environment,
and therefore PEP 517 isn't being used.

To add this custom logic to `setup.py`, add a new argument to
`create_basic_sdist_for_package`. Note that to make this work, I had to
switch from f-strings to `str.format`, since the `dedent` has to happen
before formatting.
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 15, 2022
@SpecLad
Copy link
Contributor Author

SpecLad commented Jun 15, 2022

Rebased to fix conflicts.

@sbidoul sbidoul merged commit 25dd005 into pypa:main Jun 24, 2022
@sbidoul
Copy link
Member

sbidoul commented Jun 24, 2022

Thank you @SpecLad !

@SpecLad SpecLad deleted the download-propagate-pep517 branch June 24, 2022 22:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specifying --use-pep517 to a non-pyproject.toml project should populate build requirements
4 participants