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

CentOS install re-worked to provide PHP7 via Remi Repository #2161

Merged
merged 2 commits into from Jun 3, 2018
Merged

CentOS install re-worked to provide PHP7 via Remi Repository #2161

merged 2 commits into from Jun 3, 2018

Conversation

bcambl
Copy link
Member

@bcambl bcambl commented May 1, 2018 •
edited

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes, and have included unit tests where possible.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits.

What does this PR aim to accomplish?:
CentOS ships with an unsupported version of PHP.
Provide installer option to update PHP to a supported version.
See: http://php.net/supported-versions.php

How does this PR accomplish the above?:
Prompt user to enable the Remi RPM repository (default to using system default PHP version)
See: https://rpms.remirepo.net

What documentation changes (if any) are needed to support this PR?:
Documentation TBD as discussion around this change is required.

stickler-ci
stickler-ci reviewed May 1, 2018
View reviewed changes
automated install/basic-install.sh Outdated
rpm -q ${REMI_PKG} &> /dev/null || rc=$?
if [[ $rc -ne 0 ]]; then
# The PHP version available via default repositories is older than version 7
if ! whiptail --defaultno --title "PHP 7 Update (recommended)" --yesno "PHP 7.x is recommended for both security and language features.\nWould you like to install PHP7 via Remi's RPM repository?\n\nSee: https://rpms.remirepo.net for more information" ${r} ${c}; then
Copy link

Choose a reason for hiding this comment

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

Backslash is literal in "\n". Prefer explicit escaping: "\n".

@bcambl
CentOS install re-worked to include PHP7 via Remi repository
8ad37af 
Signed-off-by: bcambl <blayne@blaynecampbell.com>
@bcambl
add 'which' dependency (missing on Fedora minimal)
ec3802c 
Signed-off-by: bcambl <blayne@blaynecampbell.com>
stickler-ci
stickler-ci reviewed May 4, 2018
View reviewed changes
automated install/basic-install.sh
@@ -215,16 +215,73 @@ elif command -v rpm &> /dev/null; then
UPDATE_PKG_CACHE=":"
PKG_INSTALL=(${PKG_MANAGER} install -y)
PKG_COUNT="${PKG_MANAGER} check-update | egrep '(.i686|.x86|.noarch|.arm|.src)' | wc -l"
INSTALLER_DEPS=(dialog git iproute net-tools newt procps-ng)
INSTALLER_DEPS=(dialog git iproute net-tools newt procps-ng which)
Copy link

Choose a reason for hiding this comment

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

INSTALLER_DEPS appears unused. Verify it or export it.

@bcambl
Copy link
Member Author

bcambl commented May 5, 2018

I think the bots are drunk again.. http://i.imgur.com/lj7B5sQ.gifv

@pi-hole pi-hole deleted a comment May 5, 2018
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/centos-7-5-1804-php-error/9456/4

@bcambl bcambl mentioned this pull request May 11, 2018
Closed
3 tasks
@rrobgill rrobgill mentioned this pull request May 11, 2018
Closed
3 tasks
@DL6ER
Copy link
Member

DL6ER commented May 11, 2018

Tested on Fedora 28 and CentOS 7

screenshot at 2018-05-11 17-53-17

[root@vultr .pihole]# php -v
PHP 7.2.5 (cli) (built: Apr 24 2018 19:12:06) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies

on the CentOS machine

@pi-hole pi-hole deleted a comment May 13, 2018
@dschaper dschaper added PR: Approved Open Pull Request, Approved by required number of reviewers Documentation Needed and removed Discussion labels May 13, 2018
@pi-hole pi-hole deleted a comment May 14, 2018
@AzureMarker
Copy link
Contributor

Is this waiting on anything?

@dschaper
Copy link
Member

Should be ready to go, just needs any documentation and updates to changelog. It's probably mostly self-documenting with the whiptail dialog, but it's a change to the installer so the team should know that it's being implemented.

@bcambl
Copy link
Member Author

bcambl commented May 14, 2018

Do we need to accommodate for unattended?

@dschaper
Copy link
Member

It would be nice to have, probably would involve another config line in setupVars to get around the whiptail for interactive installs.

@dschaper
Copy link
Member

dschaper commented Jun 3, 2018

Added release notes for PR

@dschaper dschaper merged commit feba4da into development Jun 3, 2018
@dschaper dschaper deleted the centos_remi-php7 branch June 3, 2018 17:54
@rrobgill rrobgill mentioned this pull request Jun 10, 2018
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stickler-ci stickler-ci stickler-ci left review comments

@DL6ER DL6ER DL6ER approved these changes

Assignees
No one assigned
Labels
Documentation Needed PR: Approved Open Pull Request, Approved by required number of reviewers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@bcambl @pralor-bot @DL6ER @AzureMarker @dschaper @stickler-ci