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

Fix build directory assertion for a PEP 517/518 world #6171

Merged
merged 3 commits into from
Jan 23, 2019

Conversation

pradyunsg
Copy link
Member

Closes #6158

When the ephemeral cache is used, the build can always occur. There is
no need to check for those.
@@ -840,12 +840,6 @@ def build(
newly built wheel, in preparation for installation.
:return: True if all the wheels built correctly.
Copy link
Member

Choose a reason for hiding this comment

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

(unrelated to this PR but the docstring seems obsolete :) )

have_directory_for_build = self._wheel_dir or (
autobuilding and self.wheel_cache.cache_dir
)
assert have_directory_for_build
Copy link
Member

Choose a reason for hiding this comment

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

For future reference, it looks like an alternative way to do this would be to calculate a needs_ephem_cache variable (or similarly defined variable) prior to the for loop above. And then check that ephem_cache is true inside the for loop prior to appending to the buildset (depending on whether needs_ephem_cache is true). That would give you more information if the condition ever fails, and let you raise an exception with more specific, contextual info (as well as fail faster).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, this worked and I decided "good is better than perfect" wrt code cleanup here. :)

@pradyunsg pradyunsg merged commit ab79665 into pypa:master Jan 23, 2019
@pradyunsg pradyunsg deleted the fix/pep-517-building-assertion branch January 23, 2019 12:16
simingy added a commit to CiscoTestAutomation/pyats-docker that referenced this pull request Jan 24, 2019
@simingy
Copy link

simingy commented Jan 24, 2019

still seeing this issue in v19.0.1 in alpine:3.8 when pip installing using --no-cache-dir.

adding --no-use-pep517 seems to be a workaround

@lock
Copy link

lock bot commented May 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assertion when using --no-cache-dir in 19.0
6 participants