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.

Sign up for GitHub

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

Jump to bottom

Do not download editables while preparing requirements #9123

Merged
merged 3 commits into from
Nov 22, 2020
Merged

Do not download editables while preparing requirements #9123

merged 3 commits into from
Nov 22, 2020

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Nov 11, 2020 •
edited
Loading

Fixes #9122

@sbidoul sbidoul force-pushed the pip-wheel-editable-fix-sbi branch from 5760ea8 to 8556c95 Compare November 12, 2020 11:37
@sbidoul sbidoul force-pushed the pip-wheel-editable-fix-sbi branch 2 times, most recently from e91e127 to b0baedf Compare November 12, 2020 12:23
@sbidoul
Copy link
Member Author

sbidoul commented Nov 12, 2020

So 3 things here:

  • A new test that pip download -r requirements.txt actually generate zip files for editable requirements. That feature may be questionable (and it is actually broken when the download directory is a subdirectory of an editable directory) but that is what pip 20.2.4 does and the only explanation I see for the archive() call in prepare_editable_requirements.
  • A new test that pip wheel of an editable requirement does not generate a .zip file in the target directory.
  • Fix pip wheel of editable requirements creates a .zip file in the wheel directory #9122 by removing the archive() call from prepare_editable_requirement and moving it late in the download command (via save_linked_requirement)

@sbidoul sbidoul marked this pull request as ready for review November 12, 2020 12:35
@sbidoul sbidoul mentioned this pull request Nov 12, 2020
Closed
pradyunsg
pradyunsg approved these changes Nov 20, 2020
View reviewed changes
news/9122.bugfix.rst Outdated Show resolved Hide resolved
@pradyunsg pradyunsg added this to the 20.3 milestone Nov 20, 2020
@sbidoul
Copy link
Member Author

sbidoul commented Nov 20, 2020

Thanks for the review and fixes, @pradyunsg !

pradyunsg
pradyunsg reviewed Nov 20, 2020
View reviewed changes
tests/functional/test_download.py Outdated
"""
editable_path = data.src / 'simplewheel-1.0'
requirements_path = tmpdir / "requirements.txt"
requirements_path.write_text("-e " + str(editable_path.resolve()) + "\n")
Copy link
Member

Choose a reason for hiding this comment

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

ERROR: R:Temppytest-of-VssAdministratorpytest-1popen-gw1test_download_editable0datasrcsimplewheel-1.0 is not a valid editable requirement. It should either be a path to a local project or a VCS URL (beginning with svn+, git+, hg+, or bzr+).

@pfmoore @uranusjr would you have any idea why os.path.realpath that's running behind this call is messing up the contents of this file?

Copy link
Member

@pradyunsg pradyunsg Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is happening only on the Windows runners, and "it works on my machine". 😅

@sbidoul sbidoul force-pushed the pip-wheel-editable-fix-sbi branch 2 times, most recently from 7a4b91e to 30fe899 Compare November 22, 2020 18:30
@pradyunsg
Copy link
Member

Can you add a print so that we get the value from the CI? And, maybe rebase on master, just-in-case?

@sbidoul
Do not download editables while preparing requirements
a24d198 
Downloading is done at the end of the download command
just like any other requirement. This is necessary to avoid
archiving editable requirements to a zip file when running
pip wheel.
@sbidoul sbidoul force-pushed the pip-wheel-editable-fix-sbi branch from 30fe899 to a24d198 Compare November 22, 2020 20:14
@sbidoul
Copy link
Member Author

sbidoul commented Nov 22, 2020

Looks like paths must be have forward slashes in req files. There is another test doing that here.

@sbidoul
Copy link
Member Author

sbidoul commented Nov 22, 2020

So it's 🟢 now :)

@pradyunsg pradyunsg merged commit aa847ea into pypa:master Nov 22, 2020
@pradyunsg
Copy link
Member

\o/

@sbidoul sbidoul deleted the pip-wheel-editable-fix-sbi branch November 23, 2020 18:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
20.3
Development

Successfully merging this pull request may close these issues.

pip wheel of editable requirements creates a .zip file in the wheel directory
2 participants
@sbidoul @pradyunsg