Skip to content
This repository was archived by the owner on Oct 2, 2018. It is now read-only.

[COOK-3401] Add Option to 'make altinstall' After Compiling Python From Source#52

Closed
tjcravey wants to merge 9 commits intopoise:masterfrom
tjcravey:master
Closed

[COOK-3401] Add Option to 'make altinstall' After Compiling Python From Source#52
tjcravey wants to merge 9 commits intopoise:masterfrom
tjcravey:master

Conversation

@tjcravey
Copy link
Copy Markdown

When using the source install option by setting the attribute node['python']['install_method'] as documented in the readme to 'source' the compiled binaries are installed with 'make install' which overwrites the OS's system python. For yum-based Linux distributions, this can be problematic because yum on current versions of RHEL and CentOS has dependencies on Python 2.6. The current default Python installed when only setting the node['python']['install_method'] to source is Python 2.7.5.

Installing with 'make altinstall' instead of 'make install' allows the new version of Python to be installed alongside the already installed version. Further, installing setuptools and pip with the altinstall'd binary allows python packages installed with the python_pip resource in this cookbook to target this version and virtualenvs created with the python_virtualenv resource in this cookbook are able to target this version with the interpreter attribute.

recipes/pip.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This still seems like the wrong approach, if what you want is /usr/bin/python2.7, then the binary should be set to that. I don't like the duplication of having these twice when it should only matter for one. If what you want is the ability to manage multiple versions of Python with Chef then I think the best I can say is "stay tuned" :)

README.md Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typographical error on "attirbute"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@btm
Copy link
Copy Markdown
Contributor

btm commented Aug 29, 2013

@tjcravey Can you please look at rebasing this against master? The above "We can’t automatically merge this pull request." message indicates there is a merge conflict. Perhaps this is due to PR #54 finally getting merged in. Did you get a chance to look at this @akiernan?

@coderanger
Copy link
Copy Markdown
Member

You need to set a default value for install_type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This feels like one too many variables to me - isn't it really just another install method - package, source, source-altinstall? Right now if you were to typo install_type you'd get a random command line passed into the make install step which might not fail, but leave you with a real headscratcher as to why Python's not where you thought it would be...

@akiernan
Copy link
Copy Markdown
Contributor

IMO we need coverage of altinstall in the tests - probably as simple as copying the existing source test and updating the attributes to do an altinstall.

@sethvargo
Copy link
Copy Markdown
Contributor

@tjcravey this cannot be merged cleanly. Could you please rebase?

@pdunnavant
Copy link
Copy Markdown

Hi there! Is there any way that I can help with this? I've actually been waiting for this method of installing python as well. Maybe this will just serve as a bump, but I'd be happy to do the rebase if that helps to move things along with this PR. Thanks!

@coderanger
Copy link
Copy Markdown
Member

As it stands this cannot be merged until the default value for install_type is added.

@pdunnavant
Copy link
Copy Markdown

In the code that @tjcravey has currently, it looks like install_type is only used when install_method != 'package'. There's a check in the attributes that sets a default for install_type when not using the package install_method. Is that sufficient or are you thinking of maybe a different way of doing this?

@coderanger
Copy link
Copy Markdown
Member

Yes, and if that is the case and install_type is not set, this code will fail.

@pdunnavant
Copy link
Copy Markdown

Ok, I think that makes sense. Then, more in line with what akiernan mentioned above, would something like this work better? I added a private recipe and have the source and source-altinstall recipes reuse that. This keeps the install_type attribute from being exposed at all to the end user (since that's not really something I'd imagine they should need to muck with).
https://github.com/pdunnavant/python/compare/master...altinstall-recipe

@sethvargo
Copy link
Copy Markdown
Contributor

Closing in favor of #56

@sethvargo sethvargo closed this Oct 9, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants