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

Add an option to allow disabling build isolation #5053

Merged
merged 1 commit into from
Mar 14, 2018
Merged

Add an option to allow disabling build isolation #5053

merged 1 commit into from
Mar 14, 2018

Conversation

pradyunsg
Copy link
Member

Fixes #5033

@pradyunsg pradyunsg added type: enhancement Improvements to functionality skip news Does not need a NEWS file entry (eg: trivial changes) labels Mar 4, 2018
@pradyunsg pradyunsg added this to the 10.0 milestone Mar 4, 2018
@pradyunsg pradyunsg self-assigned this Mar 4, 2018
@pradyunsg pradyunsg force-pushed the feature/allow-disabling-isolation branch from 80da60b to ca92001 Compare March 4, 2018 17:21
@pradyunsg pradyunsg requested a review from a team March 7, 2018 16:46
pfmoore
pfmoore approved these changes Mar 7, 2018
View reviewed changes
Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

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

Looks OK. One minor question.

Also, do we need additional docs (beyond what's auto-generated) explaining why this flag should be used (and explaining that it's a specialist thing)?

tests/functional/test_wheel.py Outdated
@pytest.mark.network
def test_pip_wheel_with_pep518_build_reqs_no_isolation(script, data):
script.pip('install', 'wheel')
script.pip('download', 'setuptools', 'wheel', '-d', data.packages)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you downloading setuptools and wheel here? Isn't the point of no-isolation that you don't need them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I overlooked it while copy pasting. Whoops! Nice catch! :)

@jakirkham
Copy link
Contributor

Thanks @pradyunsg. :) Anything we need to do to test this?

cc @msarahan @isuruf

@jakirkham jakirkham mentioned this pull request Mar 7, 2018
Merged
@pradyunsg
Copy link
Member Author

pradyunsg commented Mar 8, 2018 •
edited
Loading

Also, do we need additional docs (beyond what's auto-generated) explaining why this flag should be used (and explaining that it's a specialist thing)?

I was thinking we'd cover this as a part of #4803, separately.

@pradyunsg pradyunsg force-pushed the feature/allow-disabling-isolation branch from 1c6d353 to c08d4cc Compare March 8, 2018 18:31
@pradyunsg
Copy link
Member Author

Anything we need to do to test this?

@jakirkham if you guys could take master for a spin after this merges and give us feedback on it, that would be nice! :)

Thanks for taking the time to review this @auvipy! :)

@pradyunsg
Copy link
Member Author

@pfmoore I'll merge?

@pfmoore
Copy link
Member

pfmoore commented Mar 8, 2018

Go for it (once CI says it's clean - Travis looks good so far).

I was thinking we'd cover this as a part of #4803, separately.

Works for me - I'll cover it there.

@jakirkham jakirkham mentioned this pull request Mar 14, 2018
Closed
@pfmoore pfmoore merged commit cb696fc into pypa:master Mar 14, 2018
@jakirkham jakirkham mentioned this pull request Apr 15, 2018
Closed
@pradyunsg pradyunsg deleted the feature/allow-disabling-isolation branch May 27, 2018 10:08
@lock
Copy link

lock bot commented Jun 2, 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 Jun 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pfmoore pfmoore pfmoore approved these changes

@auvipy auvipy auvipy approved these changes

Assignees

@pradyunsg pradyunsg

Labels
auto-locked Outdated issues that have been locked by automation skip news Does not need a NEWS file entry (eg: trivial changes) type: enhancement Improvements to functionality
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

Add flag to disable build isolation
4 participants
@pradyunsg @jakirkham @pfmoore @auvipy