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

Set laravel PHP default to 8.1, fixes #4651 #4653

Merged
merged 5 commits into from
Apr 13, 2023

Conversation

rfay
Copy link
Member

@rfay rfay commented Feb 15, 2023

The Issue

Laravel 10 requires PHP 8.1:

How This PR Solves The Issue

  • Change default version in code
  • For now, change docs to show explicit --php-version

Manual Testing Instructions

  • Test with laravel
  • Test quickstart

@rfay rfay mentioned this pull request Feb 15, 2023
1 task
@github-actions
Copy link

github-actions bot commented Feb 15, 2023

@tyler36
Copy link
Collaborator

tyler36 commented Feb 15, 2023

Looks good to me.

@rfay
Copy link
Member Author

rfay commented Feb 16, 2023

Laravel test in TestDdevFullSiteSetup is using a canned laravel code setup, has to be updated,
https://github.com/drud/ddev/blob/12a9d92d7ed3ebc24afa04c038f3ff90ca43698a/pkg/ddevapp/ddevapp_test.go#L180-L195

@tyler36
Copy link
Collaborator

tyler36 commented Feb 16, 2023

That appears to be Laravel Lumen:

"php": "^7.2.5",
"laravel/lumen-framework": "^7.0"

Currently, Laravel Lumen is at the follow, so still lower requirements than Laravel 10:

"php": "^8.0",
"laravel/lumen-framework": "^9.0"

@rfay
Copy link
Member Author

rfay commented Feb 16, 2023

The preference is to use the actual upstream code, but I think it requires composer install, which typically takes too long in tests, but we can make a new tarball of Laravel 10 with composer dependencies I guess...

@rfay
Copy link
Member Author

rfay commented Feb 18, 2023

@tyler36 following the quickstart I get Laravel v9.52.0, https://ddev.readthedocs.io/en/latest/users/quickstart/#laravel

So I'm on the wrong track trying to create a new tarball for code etc. If laravel 10 has been released... is it not in composer?

@tyler36
Copy link
Collaborator

tyler36 commented Feb 20, 2023

Just ran through updated quickstart again and I'm getting 10.0.2

$ mkdir my-laravel-app
$ cd my-laravel-app
$ ddev config --project-type=laravel --docroot=public --create-docroot --php-version=8.1
$ ddev composer create --prefer-dist --no-install --no-scripts laravel/laravel -y
...
Creating a "laravel/laravel" project at "/tmp/YQMaGP"
Info from https://repo.packagist.org: #StandWithUkraine
Installing laravel/laravel (v10.0.2)
  - Downloading laravel/laravel (v10.0.2)
  - Installing laravel/laravel (v10.0.2): Extracting archive

Since that's running in a new container instance, I assume theres no composer caching issue right?

@rfay
Copy link
Member Author

rfay commented Feb 23, 2023

Oh, here's the interesting thing. It's fancy:

Cannot use laravel/laravel's latest version v10.0.3 as it requires php ^8.1 which is not satisfied by your platform.

So it ends up switching to laravel 9.5!

@rfay rfay force-pushed the 20230215_change_laravel_default_php branch from 3adcdaa to 896b25e Compare February 23, 2023 02:57
@rfay rfay force-pushed the 20230215_change_laravel_default_php branch from 896b25e to 221bdec Compare April 6, 2023 22:10
@rfay rfay requested review from a team as code owners April 6, 2023 22:10
@rfay rfay requested a review from tyler36 April 6, 2023 22:11
@tyler36
Copy link
Collaborator

tyler36 commented Apr 7, 2023

Just retested this and everything appears fine.

$ ddev -v
ddev version v1.21.6-40-g221bdecb

This version results in Laravel v10.6.2 (PHP v8.1.16).

As suggested above, I think we need to add -y

- ddev composer create --prefer-dist --no-install --no-scripts laravel/laravel
+ ddev composer create --prefer-dist --no-install --no-scripts laravel/laravel -y

Without this, the installation pauses for confirmation:

$ ddev composer create --prefer-dist --no-install --no-scripts laravel/laravel
...
Warning: MOST EXISTING CONTENT in the composer root (/home/user13/temp/my-laravel-app) will be deleted by the composer create-project operation. Only .ddev, .git and .tarballs will be preserved.
Would you like to continue? [Y/n] (yes):

By including -y, I can copy/paste the entire quickstart block into my terminal and 47s later, the website opens.

docs/content/users/quickstart.md Outdated Show resolved Hide resolved
Co-authored-by: tyler36 <tyler36@users.noreply.github.com>
Copy link
Sponsor Collaborator

@mattstein mattstein left a comment

Choose a reason for hiding this comment

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

Looks good, sorry for the long wait from me!

@rfay rfay merged commit 529b33b into ddev:master Apr 13, 2023
@rfay rfay deleted the 20230215_change_laravel_default_php branch April 13, 2023 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants