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

Update letsencrypt-auto with CentOS6 SCL for python 2.7 #1172

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@linickx

linickx commented Oct 28, 2015

The attached PR fixes #1046 for CentOS 6 users by setting up python 2.7 using the Software Collection Library that is part of CentOS. (Therefore not relying on 3rd party repo's to upgrade python)

Feedback/comments/suggestions welcome.

echo "Running with virtualenv:" $SUDO $VENV_BIN/letsencrypt "$@"
$SUDO $VENV_BIN/letsencrypt "$@"
else
echo "Now run: $SUDO $SCL \"$VENV_BIN/letsencrypt $@\""

This comment has been minimized.

@kuba

kuba Oct 28, 2015

Contributor

letsencrypt-auto should call the wrapped command rather than asking user to copy paste it and run themselves

@kuba

kuba Oct 28, 2015

Contributor

letsencrypt-auto should call the wrapped command rather than asking user to copy paste it and run themselves

This comment has been minimized.

@linickx

linickx Oct 28, 2015

Yeah. I agree, however I couldn't figure it out.

My original go had $SUDO $SCL $VENV_BIN/letsencrypt "$@" (i.e. without the echo) but it fails. I tried escaping the quotes but to no avail. $SCL sets up the correct python ENV variables but something goes amiss when called in the script with SUDO... works fine if the user copy/paste.

Any idea on what I'm missing?

@linickx

linickx Oct 28, 2015

Yeah. I agree, however I couldn't figure it out.

My original go had $SUDO $SCL $VENV_BIN/letsencrypt "$@" (i.e. without the echo) but it fails. I tried escaping the quotes but to no avail. $SCL sets up the correct python ENV variables but something goes amiss when called in the script with SUDO... works fine if the user copy/paste.

Any idea on what I'm missing?

This comment has been minimized.

@pde

pde Nov 2, 2015

Member

Fun times. Try this:

$SUDO $SCL \\"$VENV_BIN/letsencrypt $@\\"

The theory is that the double escapes protect the quotes through the current shell, and the sudo layer, reconstructing them for the scl command. I don't have a Centos system to test on, but I tested in with a shell script that tries to run an analogous command through sudo bash -c \\"$@\\"

@pde

pde Nov 2, 2015

Member

Fun times. Try this:

$SUDO $SCL \\"$VENV_BIN/letsencrypt $@\\"

The theory is that the double escapes protect the quotes through the current shell, and the sudo layer, reconstructing them for the scl command. I don't have a Centos system to test on, but I tested in with a shell script that tries to run an analogous command through sudo bash -c \\"$@\\"

This comment has been minimized.

@pde

pde Nov 2, 2015

Member

Maybe if people use "$@" rather than $@ it should be $SUDO $SCL \\"$VENV_BIN/letsencrypt "$@"\\", though I think that just defers expansion of shell arguments until further down the stack?

@pde

pde Nov 2, 2015

Member

Maybe if people use "$@" rather than $@ it should be $SUDO $SCL \\"$VENV_BIN/letsencrypt "$@"\\", though I think that just defers expansion of shell arguments until further down the stack?

@kuba

This comment has been minimized.

Show comment
Hide comment
@kuba

kuba Oct 28, 2015

Contributor

What is this SCL? Is that like Debian backports? I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.

Contributor

kuba commented Oct 28, 2015

What is this SCL? Is that like Debian backports? I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.

@linickx

This comment has been minimized.

Show comment
Hide comment
@linickx

linickx Oct 29, 2015

What is this SLC. Is that like Debian backports?

SLC info can be found here:

: https://wiki.centos.org/AdditionalResources/Repositories/SCL https://wiki.centos.org/AdditionalResources/Repositories/SCL

… basically allows newer languages to be installed along side the stock ones, in a non-breaking vendor supported way.

There is some upstream scl packaging documentation is here:

: https://jnovy.fedorapeople.org/scl-utils/scl.pdf https://jnovy.fedorapeople.org/scl-utils/scl.pdf

I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.

Probably.

On 28 Oct 2015, at 22:20, Jakub Warmuz notifications@github.com wrote:

What is this SLC. Is that like Debian backports? I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.


Reply to this email directly or view it on GitHub #1172 (comment).

linickx commented Oct 29, 2015

What is this SLC. Is that like Debian backports?

SLC info can be found here:

: https://wiki.centos.org/AdditionalResources/Repositories/SCL https://wiki.centos.org/AdditionalResources/Repositories/SCL

… basically allows newer languages to be installed along side the stock ones, in a non-breaking vendor supported way.

There is some upstream scl packaging documentation is here:

: https://jnovy.fedorapeople.org/scl-utils/scl.pdf https://jnovy.fedorapeople.org/scl-utils/scl.pdf

I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.

Probably.

On 28 Oct 2015, at 22:20, Jakub Warmuz notifications@github.com wrote:

What is this SLC. Is that like Debian backports? I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR.


Reply to this email directly or view it on GitHub #1172 (comment).

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Oct 29, 2015

i'd make it optional or add a check if /usr/bin/python2.7 already exists as some folks like myself use IUS Community repo for python 2.7 and may conflict https://community.letsencrypt.org/t/redhat-centos-6-x-users-need-python-2-7/2190/

i'd only run SCL commands/install if the following didn't already exist

  • file for /etc/yum.repos.d/ius.repo and
  • /usr/bin/python2.7

there's 2 references in bootstrap/centos.sh and letsencrypt-auto to

if [ `lsb_release -rs | cut -f1 -d.` -eq 6 ]

could just become

if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ ! -f /etc/yum.repos.d/ius.repo || ! -f /usr/bin/python2.7 ]]

and set PYTHON_VER="python2.7" if these exist

  • file for /etc/yum.repos.d/ius.repo and
  • /usr/bin/python2.7

something like

if [ `lsb_release -is` == "CentOS" ]; then
    $SUDO $BOOTSTRAP/centos.sh
if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ ! -f /etc/yum.repos.d/ius.repo || ! -f /usr/bin/python2.7 ]]; then
    source /opt/rh/python27/enable
    PYTHON_VER="python2.7"
    SCL="scl enable python27 "
fi
else
    if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ -f /usr/bin/python2.7 ]]; then
        PYTHON_VER="python2.7"
    fi
    $SUDO $BOOTSTRAP/_rpm_common.sh
fi

This way if /usr/bin/python2.7 already exists on CentOS 6 system, the client doesn't need to install and setup SCL python 2.7.

FYI, IUS Community repo is maintained by rackspace.com folks and also offers python 3.4 along with python 2.7 and 3.3 which SCL doesn't offer (only python 2.7 and 3.3)

Also you may want to disable SCL repo by default and specifically enable it passing --enablerepo for yum installs as SCL contains other software as well

centminmod commented Oct 29, 2015

i'd make it optional or add a check if /usr/bin/python2.7 already exists as some folks like myself use IUS Community repo for python 2.7 and may conflict https://community.letsencrypt.org/t/redhat-centos-6-x-users-need-python-2-7/2190/

i'd only run SCL commands/install if the following didn't already exist

  • file for /etc/yum.repos.d/ius.repo and
  • /usr/bin/python2.7

there's 2 references in bootstrap/centos.sh and letsencrypt-auto to

if [ `lsb_release -rs | cut -f1 -d.` -eq 6 ]

could just become

if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ ! -f /etc/yum.repos.d/ius.repo || ! -f /usr/bin/python2.7 ]]

and set PYTHON_VER="python2.7" if these exist

  • file for /etc/yum.repos.d/ius.repo and
  • /usr/bin/python2.7

something like

if [ `lsb_release -is` == "CentOS" ]; then
    $SUDO $BOOTSTRAP/centos.sh
if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ ! -f /etc/yum.repos.d/ius.repo || ! -f /usr/bin/python2.7 ]]; then
    source /opt/rh/python27/enable
    PYTHON_VER="python2.7"
    SCL="scl enable python27 "
fi
else
    if [[ `lsb_release -rs | cut -f1 -d.` -eq 6 ]] && [[ -f /usr/bin/python2.7 ]]; then
        PYTHON_VER="python2.7"
    fi
    $SUDO $BOOTSTRAP/_rpm_common.sh
fi

This way if /usr/bin/python2.7 already exists on CentOS 6 system, the client doesn't need to install and setup SCL python 2.7.

FYI, IUS Community repo is maintained by rackspace.com folks and also offers python 3.4 along with python 2.7 and 3.3 which SCL doesn't offer (only python 2.7 and 3.3)

Also you may want to disable SCL repo by default and specifically enable it passing --enablerepo for yum installs as SCL contains other software as well

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Oct 29, 2015

@kuba

I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR

indeed... it's probably better to add the various ways centos/rhel 6 users can install python 2.7 via SCL or IUS Community repo in the official documentation and manual/wikis

centminmod commented Oct 29, 2015

@kuba

I think some people might not like the fact we enable this repo for them. I consider this a main showstopper for this PR

indeed... it's probably better to add the various ways centos/rhel 6 users can install python 2.7 via SCL or IUS Community repo in the official documentation and manual/wikis

@linickx

This comment has been minimized.

Show comment
Hide comment
@linickx

linickx Oct 30, 2015

Good suggestions 👍 I've committed an update. I can squash the commits, if the maintainers/collaborators do think this PR will be accepted.

linickx commented Oct 30, 2015

Good suggestions 👍 I've committed an update. I can squash the commits, if the maintainers/collaborators do think this PR will be accepted.

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Oct 30, 2015

cheers @linickx maybe related too #1232

centminmod commented Oct 30, 2015

cheers @linickx maybe related too #1232

@kuba

This comment has been minimized.

Show comment
Hide comment
@kuba

kuba Oct 31, 2015

Contributor

You shouldn't need to install Python 2.7 at all. This will be fixed in #1257.

Contributor

kuba commented Oct 31, 2015

You shouldn't need to install Python 2.7 at all. This will be fixed in #1257.

@pde pde changed the title from Update letsencrypt-auto with CentOS6 SCL for python 2.7 to Update letsencrypt-auto with CentOS6 SCL for python 2.7 [needs revision] Nov 2, 2015

@pde pde changed the title from Update letsencrypt-auto with CentOS6 SCL for python 2.7 [needs revision] to Update letsencrypt-auto with CentOS6 SCL for python 2.7 Nov 2, 2015

@pde

This comment has been minimized.

Show comment
Hide comment
@pde

pde Nov 2, 2015

Member

@kuba are you saying that landing #1257 will make this PR unnecessary as a whole? If so I guess there's no point in revising. Although things like #1278 suggest it might be worth doing anyway?

Member

pde commented Nov 2, 2015

@kuba are you saying that landing #1257 will make this PR unnecessary as a whole? If so I guess there's no point in revising. Although things like #1278 suggest it might be worth doing anyway?

@pde

This comment has been minimized.

Show comment
Hide comment
@pde

pde Nov 12, 2015

Member

@kuba @linickx should this PR be landed, or is it obsolete now?

Member

pde commented Nov 12, 2015

@kuba @linickx should this PR be landed, or is it obsolete now?

@bmw

This comment has been minimized.

Show comment
Hide comment
@bmw

bmw Nov 19, 2015

Contributor

Since Python 2.6 support was added to letsencrypt-auto, I believe this PR is obsolete. If you disagree, yell at me and I'll reconsider.

Contributor

bmw commented Nov 19, 2015

Since Python 2.6 support was added to letsencrypt-auto, I believe this PR is obsolete. If you disagree, yell at me and I'll reconsider.

@bmw bmw closed this Nov 19, 2015

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