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.

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

starter.ini: Make the PHP version configurable #156

Closed
apolopena opened this issue Dec 15, 2021 · 29 comments
Closed

starter.ini: Make the PHP version configurable #156

apolopena opened this issue Dec 15, 2021 · 29 comments
Assignees
Labels
enhancement New feature or request passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main

Comments

@apolopena
Copy link
Owner

apolopena commented Dec 15, 2021

Problem this feature will solve

Users are forced to use the version of PHP that the gitpods workspace-full image uses. Whenever the version of PHP changes in gitpod workkspace-full, this entire project breaks at the .Dockerfile level (when the image is built) and will not run.

Background

Currently the PHP version in workspace-full is version 8.0 which is only about 17% adopted vs the 45%-ish adoption rate of PHP 7.4

Proposed Solution

Allow the PHP version to be set from starter.ini in the [PHP] section.
Acceptable values should be:
7.4
gitpodlatest

  • The starter.ini value 7.4 will install PHP 7.4.* in addition to the latest PHP version and packages as per the gitpod image workspace-full with the following PHP packages:
    • php7.4 php7.4-fpm php7.4-dev php7.4-bcmath php7.4-ctype php7.4-curl php-date php7.4-gd php7.4-intl php7.4-json php7.4-mbstring php7.4-mysql php-net-ftp php7.4-pgsql php7.4-sqlite3 php7.4-tokenizer php7.4-xml php7.4-zip
  • The `latest' value will install the just version of PHP and its respective packages as per the gitpod image workspace-full

The default value in starter.ini should be this:

[PHP]
version: 7.4

Also dynamically set the PHP API (extension_dir) xdebug paths and anything else that needs the PHP API path (such as the server function start_server). The extensions path for PHP on a debian system can be dynamically obtained with the following command:

php -r 'echo ini_get("extension_dir");'

Constraints and Assumptions

The Installs for PHP happen at the .gitpod..Dockerfile (Docker) level so the PHP version in starter.ini will be cached! This means that any change to the PHP version in starter.ini will require the docker cache to be cleaned by incrementing the INVALIDATE_CACHE value in .gitpod..Dockerfile

Depends on #172

Alternatives or Workarounds

None worth mentioning

Additional context

Since the PHP extension path (extension_dir) will now derived dynamically nothing should break when Gitpod updates the version of PHP.

@apolopena
Copy link
Owner Author

apolopena commented Dec 15, 2021

Yeah we can grab the PHP API version (dated folder name of the PHP version currently in use) and derive the extension_path with this:

echo /usr/lib/php/"$(php -i | grep 'PHP API' | sed -e 's/PHP API => //')"

Even better, instead of building out the assumed extension path we can grab it directly:

php -r 'echo ini_get("extension_dir");'

@apolopena
Copy link
Owner Author

apolopena commented Dec 15, 2021

xdebug extension in .gitpod.Dockerfile can be set dynamically:

echo -e "\nzend_extension = $(php -r 'echo ini_get("extension_dir");')/xdebug.so\n[XDebug]\nxdebug.client_host = 127.0.0.1\nxdebug.client_port = 9009\nxdebug.log = /var/log/xdebug.log\nxdebug.mode = debug\nxdebug.start_with_request = trigger\n"

I am looking into consolidating .gp/conf/xdebug.ini so everything is cleaner and more concise since currently .gitpod.Dockerfile writes out the same thing that is in .gp/conf/xdebug.ini

Also since the extension path is dynamically derived it now makes sense to generate xdebug.ini on-the-fly and do away with .gp/conf/xdebug.ini

apolopena added a commit that referenced this issue Dec 16, 2021
@apolopena
Copy link
Owner Author

This approach will also be used in the ioncube install in init-gitpod.sh hencing making that feature futureproof against PHP version changes in the gitpod-full image.

@strausmann
Copy link
Contributor

IonCube does not have PHP 8 support yet. Therefore we have to make sure that ioncube is currently only rolled out if php switch is on 7.4.

@apolopena
Copy link
Owner Author

apolopena commented Dec 20, 2021

I will make a note of that in the ioncobe comments in starter.ini and be sure to not install ioncube in init-gitpod.sh if php8+/latest is in use

@apolopena apolopena added this to the Version 1.4🚀 RELEASE milestone Dec 20, 2021
apolopena added a commit that referenced this issue Dec 21, 2021
apolopena added a commit that referenced this issue Dec 22, 2021
apolopena added a commit that referenced this issue Jan 20, 2022
@apolopena
Copy link
Owner Author

apolopena commented Jan 21, 2022

OK so I have an optional dynamic install of a supported (as per starter.ini) PHP version working now. It was not easy mostly because PHP is installed as a module for Apache.

Now that the optional dynamic install of a PHP version works though, ppa:ondrej/php is used which I dont think should be forced onto the user. Rather the user should have the option of using the standard OS PPA for their version and this should be the default.

ppa:ondrej/php is currently used because this is what is in the underlying image we use: https://github.com/gitpod-io/workspace-images/blob/master/full/Dockerfile

We will continue to use https://github.com/gitpod-io/workspace-images/tree/master/mysql which is built upon https://github.com/gitpod-io/workspace-images/blob/master/full/Dockerfile so that we can get all the latest updates from Gitpod, hoever we we now have the option to pin any supported PHP version we like and we should be able to control the PPA used for that.

With all this said, I will open another feature that allows to user to either install their optional PHP version using the OS version PPA (as the default) or to use ppa:ondrej/php (which is what the gipod workspace image is currently using).

Lastly I do have an issue logged with Gitpod that questions their usage of ppa:ondrej/php in the first place.

@apolopena apolopena changed the title Configure PHP version in starter.ini starter.ini: Make the PHP version configurable Jan 21, 2022
@apolopena
Copy link
Owner Author

Depends on #172

@apolopena apolopena added passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main and removed in-dev-qa labels Jan 22, 2022
@apolopena apolopena self-assigned this Jan 22, 2022
@apolopena apolopena added in-development Bug or feature is currently being worked on and removed passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main labels Jan 22, 2022
@apolopena
Copy link
Owner Author

swapping latest to gitpodlatest for the sake of clarity

@strausmann
Copy link
Contributor

good idea

@apolopena
Copy link
Owner Author

I will make a note of that in the ioncobe comments in starter.ini and be sure to not install ioncube in init-gitpod.sh if php8+/latest is in use

Also working this in now.

apolopena added a commit that referenced this issue Jan 22, 2022
@apolopena
Copy link
Owner Author

ioncube version mismatch ready to test in development

@strausmann
Copy link
Contributor

what exactly is the default php version for now? 7.4 or ?

@apolopena apolopena added in-dev-qa and removed in-development Bug or feature is currently being worked on labels Jan 22, 2022
@apolopena
Copy link
Owner Author

yeah 7.4 since it is currently more widely used out in the wild. Once the numbers point the other way I will change it in a future release.

@strausmann
Copy link
Contributor

the xdebug config must not be included directly in php.ini. xdebug must always be loaded after the ioncube.

image

therefore xdebug should always be integrated with the 20-xdebug.ini inside the conf.d folder

otherwise you will always get this error when running php.

image

@strausmann
Copy link
Contributor

Must still be checked for true not false.

if [[ $(bash .gp/bash/utils.sh comp_ver_lt "$current_php_version" 7.4) == 0 ]]; then

@apolopena
Copy link
Owner Author

OK I will fix this today

@apolopena
Copy link
Owner Author

apolopena commented Jan 23, 2022

This is one of those oddly flipped values since the function returns false if the first value is greater than the second but we want that reversed so after testing I think this is the line to use:

[[ $(bash .gp/bash/utils.sh comp_ver_lt "$current_php_version" "8.0") == 1 ]]

@strausmann
Copy link
Contributor

strausmann commented Jan 23, 2022

This Line is corret

$(xdebug_zend_ext_conf)" > "/etc/php/$php_version/mods-available/20-xdebug.ini"

But this are the problem.

&& sudo bash -c "echo -e \"$(xdebug_zend_ext_conf)\" >> \"/etc/php/$php_version/cli/php.ini\"" \

&& sudo bash -c "echo -e \"$(xdebug_zend_ext_conf)\" >> \"/etc/php/$php_version/apache2/php.ini\"" \

@apolopena
Copy link
Owner Author

yeah ok thanks for clarifing. I thought so. I will change them to conf.d rather than directly in the php.ini

@strausmann
Copy link
Contributor

and I think this will not work with a PHP8 workspace either.

&& /usr/bin/phpize7.4 \

Would have to be phpize8.0

@apolopena
Copy link
Owner Author

Yeah

and I think this will not work with a PHP8 workspace either.

&& /usr/bin/phpize7.4 \

Would have to be phpize8.0

Yeah I will make that dynamic

apolopena added a commit that referenced this issue Jan 23, 2022
@apolopena apolopena added in-development Bug or feature is currently being worked on and removed in-dev-qa labels Jan 23, 2022
@apolopena
Copy link
Owner Author

alright its close. xdebug and ioncube gets properly installed fro php 7.4 all dynamically. Dynamic phpize works too.

Retaining the existing php version using an invalid value or gitpodlatest is not working (its installing 7.4 no matter what) and the error: about php_fpm_conf or whtever it was is still occuring.

I have to step away for the rest of the day. I will pick this back up this evening or tomorrow morning.

@apolopena apolopena added in-dev-qa and removed in-development Bug or feature is currently being worked on labels Jan 23, 2022
apolopena added a commit that referenced this issue Jan 24, 2022
@apolopena
Copy link
Owner Author

Test success
ioncube will not be installed if php version > 7.4
image

@apolopena
Copy link
Owner Author

Test success
System falls back to gitpodlatest version of PHP if an invalid/unsupported value is set in starter.ini
image

@apolopena
Copy link
Owner Author

apolopena commented Jan 24, 2022

Test success
The appropriate version of php-fpm is installed in install-core-packages.sh if the currently installed version of PHP greater than 7.4
image

@apolopena
Copy link
Owner Author

Test success
php 7.4 and php7.4-fpm is successfully installed. existing version of php ('gitpodlatest') is purged and ppa:ondrej/php is remove when ppa=OS (and version is less than 8.0) in starter.ini. ioncube is also successfully installed when [ioncube] install=1
xdebug is also properly downloaded compiled installed (and dynamically) php, php cli, and php-fpm
image
image
image

@apolopena
Copy link
Owner Author

apolopena commented Jan 24, 2022

@strausmann
I accidentally commited the built version of the project from gitpod which makes quite a mess.
I had to force revert the development branch to commit cf01bdf180cc0927655461fffd853d97969f0fd7

Long story short. Please make sure you create a new development branch in your fork to avoid any issue when you test.

Feel free to test the items I mentioned above with screenshots if you like.
Any easy way to test and branch of this repo as a new repo of your own is to use this script:
https://gist.github.com/apolopena/8dcbde7dd0c0cec1cb1e11c15a5e19ff
Following the general directions found here:
https://github.com/apolopena/gitpod-laravel-starter/wiki/Setup#the-developer
This allow you to not be locked to a single fork when you are testing or contributing.

Lastly I will work on the last remaining issues #170 and #176 now. Hopefully all this dynamic php installation and config, ppa, ioncube and xdebug is solid now. Looks that way to me so far.

@apolopena apolopena added passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main and removed in-dev-qa labels Jan 24, 2022
@strausmann
Copy link
Contributor

@apolopena
all your changes look very good to me. i have not been able to find any errors now either.
Thank you for your great work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main
Projects
None yet
Development

No branches or pull requests

2 participants