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

Internal utility to set the PHP version of VVV #2309

Closed
Mte90 opened this issue Nov 27, 2020 · 4 comments
Closed

Internal utility to set the PHP version of VVV #2309

Mte90 opened this issue Nov 27, 2020 · 4 comments

Comments

@Mte90
Copy link
Member

Mte90 commented Nov 27, 2020

The idea behind this is to avoid issues when we change the internal default php version.
AS we need to change various files and version in a lot of files, something that does this for us automatically can save a lot of time.

@tomjn
Copy link
Member

tomjn commented Nov 27, 2020

It will need to:

  • read in from the config file what the preferred default PHP should be
  • check that it's available
    • This means it will need to run after the utilities have run else we could declare we want 7.0 to be the default and it would not find it because it hasn't been installed yet
  • The dashboard/XHGUI/PHPinfo/etc currently use php upstream, but these should be changed instead to an internal upstream that uses our preferred default so that changing the default doesn't break these

The last item can be spun off into a new ticket

@tomjn tomjn added this to the 3.x.x milestone Nov 27, 2020
@Mte90 Mte90 self-assigned this Dec 16, 2020
@Mte90
Copy link
Member Author

Mte90 commented Jan 27, 2021

Reading now the ticket seems that we need as always that in VVV define a PHP default version in case the one in the config is not available.

@Mte90
Copy link
Member Author

Mte90 commented Jan 21, 2022

So as today we have

VVV_BASE_PHPVERSION=${VVV_BASE_PHPVERSION:-"7.4"}
this variable that is set as 7.4 we can think to a new config:

general:
  default_php_version: 8.0

So the code as wrote #2309 (comment) require some check.

Instead how this change can affect utilities (apart the php version running as default for the webserver for them):

So a solution is to enforce the VVV's php default version as minimum for web server and also for cli.
This can open to:

general:
  default_php_version_cli: 8.0
  default_php_version_nginx: 7.4

In this way we can have both the solution or another solution that add a lot of entropy is to define a php version for provisioner used for cli/nginx.

@tomjn
Copy link
Member

tomjn commented Jan 21, 2022

I'm heavily opposed to setting a default version, there are other solutions to that problem that are superior. I've explained elsewhere that this will cause lots of problems, while never actually solving the problems that people are using this to solve.

This is an X Y problem, this is not the solution to the problem, this is the solution to implementing a proposed solution that does not actually solve the real problem.

The real solution is to allow a site to specify the PHP version, not the nginx upstream, then set that version in the provisioner:

sites:
  wordpress-one:
    repo: https://github.com/Varying-Vagrant-Vagrants/custom-site-template.git
    php: 8.0
    hosts:
      - one.wordpress.test

Now when wordpress-one is provisioned its provisioner runs code in PHP 8.0, and the site is served with 8.0. This avoids a lot of problems that what you're proposing would introduce.

IMO this should be closed in favor of #2367

@tomjn tomjn closed this as completed Jan 21, 2022
@Mte90 Mte90 removed their assignment Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants