Varying-Vagrant-Vagrants / VVV

An open source Vagrant configuration for developing with WordPress
https://varyingvagrantvagrants.org
MIT License
4.55k stars 849 forks source link

Bug: Setting PHP Version in Site Config is not Working Propery #2643

Closed dwashko closed 1 year ago

dwashko commented 1 year ago

Are you using the latest stable or develop branch version of VVV?

Yes (develop)

Is it a new VVV or an existing VVV that used to work?

New fresh install

Whats the problem?

Setting the PHP version for a custom site in config.yml is not actually running that version of php. The code sets the cli to run at that version but does not set NGINX to run that version of PHP. If you look at the resulting custom site configuration file you will see that $upstream is being set to the default version of PHP VVV uses. I believe the problem start with https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/provision-site.sh#L546To#L547 Where

source /srv/config/homebin/vvv_restore_php_default
VVV_DEFAULTPHP=$DEFAULT_PHP

DEFULTPHP is set by vvv_restore_php_default being sourced. DEFUALT_PHP is not set as far as I can tell so the value of VVV_DEFAULTPHP is always empty.

Then when it is going to set the value of PHP to be used in the custom nginx site: https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/provision-site.sh#L160ToL163

  if [ "${DEFAULTPHP}" != "${VVV_DEFAULTPHP}" ]; then
    NGINX_UPSTREAM="php${DEFAULTPHP}"
    NGINX_UPSTREAM=$(echo "$NGINX_UPSTREAM" | tr --delete .)
  fi

It will always use DEFAULTPHP which is not the value set for the custom site because VVV_DEFAULTPHP is not set. I believe this should actually be:

 if [ "${SITE_PHP}" != "${DEFAULTPHP}" ]; then
   NGINX_UPSTREAM="php${SITE_PHP}"
   NGINX_UPSTREAM=$(echo "$NGINX_UPSTREAM" | tr --delete .)
  fi

I have run these changes locally and I can verify that my wordpress site is now using php8.0 as opposed to the default php7.4. If this looks correct I can submit a PR to fix this otherwise would someone please tell me how we are configuring VVV incorrectly and not able to set the version of PHP we want to use?

How do we reproduce it?

Create a custom site configuration and set php: 8.0 (or anything other than 7.4). SSH into the resulting vagrant box and look at the sites custom nginx file. It will show set $upstream 7.4 and not set $upstream 8.0. The custom site will not be running the specified version of PHP and instead be using 7.4.

VVV Status screen

__ __ __ __
\ V\ V\ V / v3.11 Ruby:2.6.6, Path:"/Users/dwashko/mygitstuff/story-editor"
 \_/\_/\_/  git::develop(28352878)

Platform: darwin19.2.0 shell:/bin/bash vagrant-goodhosts shared_db_folder_disabled
Vagrant: v2.2.14, virtualbox: v6.1.32

Which Operating System are you using?

Apple MacOS (Intel)

Which provider are you using?

VirtualBox

tomjn commented 1 year ago

Do you have PHP 8 installed via the extensions?

note that the immediate line after your quote is:

VVV_DEFAULTPHP=$DEFAULT_PHP
vvv_apply_site_php_cli_version

VVV_DEFAULTPHP is supposed to hold the default PHP version, not the site specific version.

This seems to be both the culprit and the fix from what you shared though:

if [ "${SITE_PHP}" != "${DEFAULTPHP}" ]; then
tomjn commented 1 year ago

I've tried to edit and fix the code blocks ( you'd used single backticks for inline code rather than triple backticks to start a code block.

What confuses me is:

It will show set $upstream 7.4 and not set $upstream 8.0.

Our upstreams are named php80 or php74 with php being a catch all fallback for the default.

tomjn commented 1 year ago

See:

    # Upstream to abstract backend connection(s) for PHP.
    upstream php {
        server unix:/var/run/php7.4-fpm.sock;
    }

    include /etc/nginx/upstreams/*.conf;

    map '' $upstream {
        default php;
    }

https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/core/nginx/config/nginx.conf#L134-L143

And each PHP provisioner installs a file like this:

https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/config/php-config/upstream.conf

# Upstream to abstract backend connection(s) for PHP.
upstream php74 {
    server unix:/var/run/php7.4-fpm.sock;
}

7.4 and 8.0 were never valid upstreams

GitHub
VVV/nginx.conf at develop · Varying-Vagrant-Vagrants/VVV
An open source Vagrant configuration for developing with WordPress - VVV/nginx.conf at develop · Varying-Vagrant-Vagrants/VVV
GitHub
VVV/upstream.conf at develop · Varying-Vagrant-Vagrants/VVV
An open source Vagrant configuration for developing with WordPress - VVV/upstream.conf at develop · Varying-Vagrant-Vagrants/VVV
tomjn commented 1 year ago

@dwashko can you share the config entry for the site in config.yml that you're using? Are you using the standard custom site template provisioner? And what does the log output for that site look like when it provisions?

dwashko commented 1 year ago

in my site config I have

sites:
  mysite:
    repo: https://github.com/Varying-Vagrant-Vagrants/custom-site-template.git
    php: 8.0
    custom:
      wp_version: 6.0.3
    hosts:
      - gannett.test
    folders:
      public_html/wp-content/:
        git:
          repo: git@github.com:wpcomvip/mysite-com.git
          overwrite_on_clone: true
      public_html/wp-content/mu-plugins:
        git:
          repo: https://github.com/Automattic/vip-go-mu-plugins.git
          overwrite_on_clone: true
          hard_reset: true
          pull: true

and under extensions have:

extensions:
  core: # The core VVV extensions
    - tls-ca # HTTPS SSL/TLS certificates
    - phpmyadmin # Web based database client
    #- memcached-admin # Object cache management
    #- opcache-status # opcache management
    #- webgrind # PHP Debugging
    #- mongodb # needed for Tideways/XHGui
    #- tideways # PHP profiling tool, also installs xhgui check https://varyingvagrantvagrants.org/docs/en-US/references/tideways-xhgui/
    #- nvm # Node Version Manager
    #- php56
    #- php70
    #- php71
    #- php72
    #- php73
    #- php74
    - php80

I was incorrect in my initial filing and the value being site in the custom nginx site looks like this:

     # This is needed to set the PHP being used
    set          $upstream php74;

Additionally, should this conditional be used?

 if [ "${SITE_PHP}" != "${DEFAULTPHP}" ]; then
   NGINX_UPSTREAM="php${SITE_PHP}"
   NGINX_UPSTREAM=$(echo "$NGINX_UPSTREAM" | tr --delete .)
  fi

Should it not always replace:

set          $upstream {upstream};

With the value of $SITE_PHP? Otherwise it would leave that line in as is in the nginx site configuration.

VVV
Tideways and XHGui
Documentation for the VVV local developer environment.
dwashko commented 1 year ago

Where is DEFAULT_PHP set? I cannot seem to find it? Both vvv_restore_default_php.sh and extenstions/core/php80/provision.sh are setting DEFAULTPHP.

tomjn commented 1 year ago

it isn't set, I have a change I'm currently provisioning to test that may or may not fix it. I suspect it was assumed that the variable defined in the restore default PHP script would be what set it but that isn't the case

dwashko commented 1 year ago

So ultimately wouldn't SITE_PHP be the value for set $upstream {upstream} That should either be the custom php version or the default php version based upon SITE_PHP=$(vvv_get_site_config_value 'php' "${DEFAULTPHP}")

tomjn commented 1 year ago

@dwashko can you test https://github.com/Varying-Vagrant-Vagrants/VVV/pull/2644 and confirm things work as intended?

dwashko commented 1 year ago

testing now

dwashko commented 1 year ago

Works perfectly for me. Thanks!