search

voxpupuli / puppet-python

Puppet module for installing and managing Python, pip, virtualenvs and Gunicorn virtual hosts.
https://forge.puppetlabs.com/puppet/python
Apache License 2.0
199 stars 370 forks source link

fix site-packages parameter for focal fossa #550

Closed crazymind1337 closed 3 years ago

crazymind1337 commented 4 years ago

Pull Request (PR) description

On Ubuntu 20.04 there is no parameter --no-site-packages for virtualenv binary anymore. The --system-site-packages stays the same.

This Pull Request (PR) fixes the following issues

Fixes '--no-site-packages' parameter on Ubuntu Focal Fossa

root@6e6956e915f1:/# grep -i focal /etc/os-release 
VERSION="20.04 LTS (Focal Fossa)"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
root@6e6956e915f1:/# virtualenv --version
virtualenv 20.0.17 from /usr/lib/python3/dist-packages/virtualenv/__init__.py
root@6e6956e915f1:/# virtualenv --help | grep 'no-site-packages'
root@6e6956e915f1:/# 
root@6e6956e915f1:/# virtualenv --help | grep '^[\ -]*system-site-packages'
  --system-site-packages        give the virtual environment access to the system site-packages dir (default: False)
igalic commented 4 years ago

and this doesn't affect any other Debian type OS?

crazymind1337 commented 4 years ago

and this doesn't affect any other Debian type OS?

Ah you are right. Just did check for our needs. I will check also for other OSes later this day. Thanks for reminding!

crazymind1337 commented 4 years ago

@igalic I checked other supported OSes (using latest docker container). At Debian the virtualenv binary is not used and on latest CentOS virtualenv does still have the --no-site-packages option.

saz commented 3 years ago

I've missed this PR and made a new one: https://github.com/voxpupuli/puppet-python/pull/552

I think it doesn't make sense to check for any OS version, as --no-site-packages is deprecated since a long time. (see https://virtualenv.pypa.io/en/16.7.9/reference.html) The default has changed to not use site-packages anyways, so we don't need to be explicit here.

crazymind1337 commented 3 years ago

I've missed this PR and made a new one: #552

I think it doesn't make sense to check for any OS version, as --no-site-packages is deprecated since a long time. (see https://virtualenv.pypa.io/en/16.7.9/reference.html) The default has changed to not use site-packages anyways, so we don't need to be explicit here.

So i can close this one?

saz commented 3 years ago

@crazymind1337 Good question.

@igalic What do you think?

igalic commented 3 years ago

i think we've accumulated a lot more feedback on what the best approach to solving this issue is in @saz's pr, so let's keep going there