pin --never-download to TRUE #412

Merged
merged 4 commits into from Mar 30, 2013

Conversation

Projects
None yet
3 participants
Owner

pfmoore commented Mar 24, 2013

pin --never-download to TRUE
attempting to use virtualenv.py in isolation will fail

@qwcode qwcode referenced this pull request in pypa/pip Mar 24, 2013

Closed

pip 1.4 #859

As an alternative, you can provide your own versions of setuptools,
distribute and/or pip on the filesystem, and tell virtualenv to use
-those distributions instead of downloading them from the Internet. To
+those distributions instead of the ones in ``virtualenv_support``. To
@qwcode

qwcode Mar 24, 2013

Contributor

as we know, --extra-search-dir doesn't actually work as advertised. #327
pretty bad that we keep advertising this flag, and make people waste time on it.

@pfmoore

pfmoore Mar 24, 2013

Owner

Yea, that's one reason I hate depending on distribute_setup/ez_setup. If and when we install from wheels, this issue will go away, but until they you're right. (I had forgotten about this problem when I made this change, TBH).

Contributor

qwcode commented Mar 24, 2013

can we add a test for this?
I just went thru and traced the logic to feel comfortable that this works, but better to have a test of course.

@@ -547,9 +547,7 @@ def _install_req(py_executable, unzip=False, distribute=False,
remove_from_env.append('PYTHONPATH')
elif never_download:
@qwcode

qwcode Mar 24, 2013

Contributor

what's to stop someone from setting --never-download to false?
and then it going out to pypi?

@pfmoore

pfmoore Mar 24, 2013

Owner

Is that possible? I didn't realise that :-( If that's the case, I'll have to rework the patch to ignore the actual value of --never-download. At which point, it might be worth just removing it and getting it over with. That would break scripts that set it, though, including at least one of mine, so I'm more reluctant to do that.

Update: it isn't possible to do so interactively, but maybe you can with environment variables or an ini file. And you certainly could in adjust_options. So I guess that needs changing.

@qwcode

qwcode Mar 25, 2013

Contributor

oh yea, this is just a flag. : )

Contributor

qwcode commented Mar 24, 2013

I think any use of --never-download should trigger a deprecation warning
http://docs.python.org/2/library/exceptions.html#exceptions.DeprecationWarning

and then a few versions later, we can remove it all together.

Owner

pfmoore commented Mar 24, 2013

Hmm, never mind, the travis build failed this time. Looks like the tests force (and rely on) never_download=True. I'd happily argue that that's a failure in the tests, but that doesn't mean much, it still needs fixing.

I'll have to have a deeper look at this.

Regarding your suggestion of a DeprecationWarning. I'm not sure that'd help much (it's suppressed by default, so no users would ever see it, and virtualenv isn't the sort of thing other projects use and test with warnings enabled...). I'm not against a deprecation policy for pip/virtualenv, but I didn't think we had one (and certainly not one this strict) at the moment. That's something that should be discussed on the list, IMO.

I think I'll probably end up abandoning this idea, at least for 1.10...

Contributor

qwcode commented Mar 25, 2013

no reason to abandon this it seems to me?

those 2 "tests" in .travis.yml that are executing an isolated "bin/virtualenv.py" can just go away.

no, there's no overt deprecation policy. details aside on how to implement it, the point would be for users to actually see a warning about the option going away in the future (and that it does nothing now). It could just a be a logger.warn message.

Owner

pfmoore commented Mar 25, 2013

OK, I was misreading the test code. I see the bit you mean in travis.yml - I've removed those tests as you suggest.

I've added a warning. For now I've just bothered about cases where the user tries (somehow) to force False, and said the option is for backward compatibility only and cannot be set to False. Next release, we can warn on any use (if I can work out how to implement that :-)) and then we can remove it after a suitable period.

BTW, what is the process for updating news.rst? Should I change it as part of this pull request? Or is it updated in the release process (which would involve less merge conflicts, but would add work to the release)?

Contributor

qwcode commented Mar 25, 2013

lgtm. Merge +1. add a test?
better to update the news now so we don't forget.
let's hold on merging, and try to get some more votes.

Contributor

pnasrat commented Mar 30, 2013

Update news and just merge - people have few enough cycles to leave pending PR's we think are good hanging up.

pfmoore added a commit that referenced this pull request Mar 30, 2013

@pfmoore pfmoore merged commit a185de7 into pypa:develop Mar 30, 2013

1 check passed

default The Travis build passed
Details
Contributor

qwcode commented Mar 30, 2013

@pnasrat, this pull is more controversial than normal, hence the idea of waiting for more votes. it was unclear that everyone wanted to do this.

Owner

pfmoore commented Mar 30, 2013

Nevertheless, I doubt I'm going to get any more comments by just waiting, so I've merged it. If anyone's using the dev version, let's see what they say. Otherwise, we can deal with feedback when it's released. People who need downloaded versions can still download manually, after all.

Contributor

qwcode commented Mar 30, 2013

I was going to shout out for more votes as the current pip/virtualenv milestones got more full.
but most likely this one would get the votes I guess.

johnsca added a commit to juju-solutions/layer-basic that referenced this pull request Mar 16, 2016

Fix use_venv on restricted-network environments
Creating venvs in network-restricted environments (on Xenial, at least) attempts to pull `setuptools` and `wheel` from pypi.  Per pypa/virtualenv#412 `--never-download` is to be set by default to address this, but isn't yet in the packaged versions, so we need to add it manually.

@johnsca johnsca referenced this pull request in juju-solutions/layer-basic Mar 16, 2016

Merged

Fix use_venv on restricted-network environments #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment