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

Release Version 4 #1318

Merged
merged 117 commits into from
Feb 20, 2023
Merged

Release Version 4 #1318

merged 117 commits into from
Feb 20, 2023

Conversation

mattstauffer
Copy link
Collaborator

@mattstauffer mattstauffer commented Dec 15, 2022

Hey, Valet 4 beta testers! Thanks for being here! Here's what I'd love you to test out for me.

Here's the upgrade doc I plan to release to the public; please let me know if it's missing anything: https://github.com/laravel/valet/blob/master/UPGRADE.md

  1. Make sure that everything works! Update to Valet v4 beta with composer global require laravel/valet:"^4.0@beta", and run at least one valet command (e.g. valet restart), and then just go about your normal life. Does it work? Do you get an errors? Let me know!
  2. Check out the new valet status command; if it returns a failure but your Valet is working correctly, let me know!
  3. Try out valet share; you'll now be prompted to install ngrok via Homebrew, or you can try out using it with Expose; I'd love for folks to test both out!
  4. If you use .valetphprc, rename the .valetphprc file to .valetrc, and prepend php= to your PHP formula; e.g. [email protected]... and test to make sure everything still works!
  5. If you have local sites on PHP 7.1-7.4, switch your linked PHP version to 8+, install Valet v4, then switch your linked version back to your older PHP (valet use [email protected]) and test to make sure everything still works correctly
  6. If you have any custom local drivers, let me know if you have any trouble getting them working with v4.

Many of the changes I made in Valet v4 are to make future extension and bug fixing simpler; there are some cool new features, but a lot of it is really architectural shifts instead, so I just want to make sure none of those shifts broke everyone's day-to-day workflows.

Thanks so much!


Original notes from this PR:

Done:


To do after v4 release:

  • Update valet status to check to make sure the certificate authority isn't expired
  • Look into @nicoverbruggen's trust CLI app: valet install asking for certificate permission for each existing site #1226
  • Build a system where the Mac-specific parts of the codebase are separate, so the Linux and Windows ports can extend this package ((WIP) Extract for multi-OS support #1323)
  • Add other powers to .valetrc
    • E.g. specify nginx includes to use when generating the Nginx config for this site
      • Would require a command that does valet apply-config or something, that e.g. re-generates nginx config files...
      • Consider: is it easier to have a potential custom stub directory for each site, either in the site or globally? Maybe allows the stubs/.valetrc type modifications to be in source control that waybasically install just for this one site
  • Default home page Marcel & Matt made
  • Consider: Plugin system suggested by Marcel (https://twitter.com/marcelpociot/status/1604603384203452416)

To document:

  • .valetrc and deprecated .valetphprc
  • set-ngrok-token command
  • New valet share that's ngrok + expose, and the valet share-tool command
  • Custom stubs https://github.com/laravel/valet/pull/1238/files
  • upgrade process to v4 (see upgrade.md)
  • 127.0.0.1 in your DNS if you want to use valet share

mattstauffer and others added 20 commits December 3, 2022 18:10
Re-work how BasicValetDriver serves files in projects with and without `public/` directory
- Extract much of server.php into a `Server` class
- Move all but the Laravel and Basic drivers into a subfolder
- Load all but the Laravel and Basic drivers via glob
- Add `beforeLoading` hook to simplify the `frontControllerPath` method for some drivers
Extract Server class and refactor loading of drivers
@pryley
Copy link

pryley commented Dec 19, 2022

Can you keep support for PHP 7.4? I use Valet for WordPress development (as I'm sure many others do also) and due to the nature of WordPress, I am forced to support a minimum of PHP 7.4.

@driesvints
Copy link
Member

@pryley PHP 7.4 is EoL: https://www.php.net/supported-versions.php

You should move away from it asap.

@pryley
Copy link

pryley commented Dec 19, 2022

@driesvints It's not a choice that I have.

With Laravel/Symfony I have the luxury of staying on the cutting edge of PHP, but WordPress is a different beast. Most of its users are hobbyists or self-taught people trying to run a website for their business as cheaply as possible, many don't have great hosting providers and wouldn't know where to start to upgrade their server software.

I develop a WordPress plugin that is used by over 50K+ websites. I don't have control of their server configuration, and as long as WordPress recommends PHP 7.4 as the minimum recommended PHP version to use, I have to still support it.

Unlike Apple, I don't have control over the ecosystem and I can't simply remove the headphone jack to speed up innovation.

image

@jools-r
Copy link

jools-r commented Dec 19, 2022

I agree. When developing a new site or project, it's not an issue but when working on legacy sites that might have constraints (e.g. scripts that are not php8 compatible or older CMS versions from which one wants to migrate) or when trying to troubleshoot / replicate other people's problems, it would be useful to still be able to use PHP7.4 for a while longer.

One of the things that makes valet so versatile is the ability to quickly switch between versions, whether via CLI or PHPmon.

@mattstauffer
Copy link
Collaborator Author

I will continue porting any important security updates/bug fixes to v3 for folks as long as they need it, but the biggest point of this refactor is to re-work the basic architecture, and add more modern coding practices to keep up with the rest of the Laravel world, and that's hard to do while I keep support for older versions of PHP.

I have long fought hard for Valet, and all Tighten packages, to work with outdated versions of PHP. So trust me, I'm here for that!

But I'm just going to recommend that anyone who works with PHP 7.* projects stick with valet 3 until they're no longer working with those projects, and to let me know if there are any really valuable features that they feel like they need in v3 that I add in v4, and I'll consider it.

@mattstauffer mattstauffer marked this pull request as ready for review February 20, 2023 03:03
@mattstauffer mattstauffer merged commit a923379 into master Feb 20, 2023
@mattstauffer mattstauffer deleted the version-4 branch February 20, 2023 03:16
@dcblogdev
Copy link

Just installed Valet all running fine so far:

CleanShot 2023-02-20 at 15 40 42@2x

Testing share:

CleanShot 2023-02-20 at 15 41 51@2x

Maybe add a prompt so users know to type y to continue

Installed ngrok added token and then did share

CleanShot 2023-02-20 at 15 49 43

but when going to the forward URL:

CleanShot 2023-02-20 at 15 50 19

Not sure if that's a user setup issue.

Trying to share via expose worked perfectly, again I have to install and add token but no issues sharing a local instance.

@muffycompo
Copy link

Software / Versions

  • OS: MacOS Ventura 13.2.1
  • Package Managers: homebrew v4.0.1, composer v2.5.4
  • PHP: v8.1.16
  • Laravel: v9.52.0
  • Valet: v4.0@beta

My test results

a) Make sure that everything works! Update to Valet v4 beta with composer global require laravel/valet:"^4.0@beta", and run at least one valet command (e.g. valet restart), and then just go about your normal life. Does it work? Do you get an errors? Let me know!

  • Installation via composer global require laravel/valet:"^4.0@beta" succeeded
  • valet restart succeeded
  • After valet restart, my projects/sites hosted via valet all loaded and are working as intended

b) Check out the new valet status command; if it returns a failure but your Valet is working correctly, let me know!

  • After issuing the valet status command, it returned a an error status but the accompanying table displayed the right information (see screenshot). I tried using the suggestion as seen in the screenshot and my local php installation got messed up (a bunch of exception was thrown) but after re-installing the php versions (8.1 & 8.2), I was able to switch between PHP 8.1 and PHP 8.2 and valet worked as intended.

screenshot 15

c) Try out valet share; you'll now be prompted to install ngrok via Homebrew, or you can try out using it with Expose; I'd love for folks to test both out!

  • I issued the valet share command and was instructed to use 'valet share-tool ngrok' or 'valet share-tool expose'. I suppose this is the expected behavior when running it for the first time (on my current machine anyway). Once I used valet share-tool ngrok, I was prompted to install ngrok (I also installed and tested expose for completeness).

screenshot 16

Valet runs as expected after installing both tools and sharing my app.

d) If you use .valetphprc, rename the .valetphprc file to .valetrc, and prepend php= to your PHP formula; e.g. [email protected]... and test to make sure everything still works!

  • I am not using .valetphprc

e) If you have local sites on PHP 7.1-7.4, switch your linked PHP version to 8+, install Valet v4, then switch your linked version back to your older PHP (valet use [email protected]) and test to make sure everything still works correctly

  • I am not using versions of PHP below 8.1 on my dev environment thankfully.

f) If you have any custom local drivers, let me know if you have any trouble getting them working with v4.

  • I am not using any custom local drivers, just the stock ones that ship with valet.

g) As an added exercise, I installed a third party tool called PHP Monitor (PHP Mon for short) and it recognized valet 4 and run successfully :)

@mattstauffer
Copy link
Collaborator Author

@dcblogdev Thanks so much! I'm not sure what that issue is with ngrok; valet share with ngrok "works on my machine" lol.. I'll see if we can get any answers whether that's a Valet issue or something about your particular machine, network, or firewall.

@muffycompo Thank you so much! And thank you for the exhaustive notes, this is very valuable!

@nicoverbruggen
Copy link
Contributor

@muffycompo Thanks for testing PHP Monitor, good to hear it isn't crashing 😁

@mattstauffer Thank you for your hard work on this large refactor, and taking the time to coordinate with me. I appreciate it! I'll give the beta a spin tomorrow as well and will report back, though I'm sure it'll work well given the current extensive test suite 👌

@caturandi-labs
Copy link

Hi Matt and Team! I love to use valet + takeout

can you make valet 4 support use shivammatur/php ? So I can use something like this with valet:

valet use shivammathur/php/[email protected]

@laytan
Copy link

laytan commented Feb 21, 2023

Hello :)

I upgraded and here are my results.

My OS/env:
Screenshot 2023-02-21 at 11 13 51

My main linked php version is 8.2.3.

  1. Make sure that everything works! Update to Valet v4 beta with composer global require laravel/valet:"^4.0@beta", and run at least one valet command (e.g. valet restart), and then just go about your normal life. Does it work? Do you get an errors? Let me know!
    Worked great, just got an error message that I needed to update my drivers but 7. is for that.
  2. Check out the new valet status command; if it returns a failure but your Valet is working correctly, let me know!
    This is not working for me, even though my sites are working, here is my output:
Fatal error: Uncaught TypeError: Valet\Status::isBrewServiceRunningGivenServiceList(): Argument #1 ($serviceList) must be of type array, null given, called in /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php on line 178 and defined in /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php:181
Stack trace:
#0 /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php(178): Valet\Status->isBrewServiceRunningGivenServiceList(NULL, 'dnsmasq', true)
#1 /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php(159): Valet\Status->isBrewServiceRunningAsUser('dnsmasq', true)
#2 /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php(94): Valet\Status->isBrewServiceRunning('dnsmasq')
#3 /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php(26): Valet\Status->Valet\{closure}()
#4 [internal function]: Valet\Status->Valet\{closure}(Array, 4)
#5 /Users/laytan/.composer/vendor/illuminate/collections/Arr.php(558): array_map(Object(Closure), Array, Array)
#6 /Users/laytan/.composer/vendor/illuminate/collections/Collection.php(768): Illuminate\Support\Arr::map(Array, Object(Closure))
#7 /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php(25): Illuminate\Support\Collection->map(Object(Closure))
#8 /Users/laytan/.composer/vendor/laravel/valet/cli/includes/facades.php(22): Valet\Status->check()
#9 /Users/laytan/.composer/vendor/laravel/valet/cli/app.php(78): Facade::__callStatic('check', Array)
#10 [internal function]: Silly\Application->{closure}(Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /Users/laytan/.composer/vendor/php-di/invoker/src/Invoker.php(74): call_user_func_array(Object(Closure), Array)
#12 /Users/laytan/.composer/vendor/mnapoli/silly/src/Application.php(96): Invoker\Invoker->call(Object(Closure), Array)
#13 /Users/laytan/.composer/vendor/symfony/console/Command/Command.php(310): Silly\Application->Silly\{closure}(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#14 /Users/laytan/.composer/vendor/symfony/console/Application.php(1040): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 /Users/laytan/.composer/vendor/symfony/console/Application.php(314): Symfony\Component\Console\Application->doRunCommand(Object(Silly\Command\Command), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 /Users/laytan/.composer/vendor/symfony/console/Application.php(168): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#17 /Users/laytan/.composer/vendor/laravel/valet/cli/valet.php(9): Symfony\Component\Console\Application->run()
#18 {main}
  thrown in /Users/laytan/.composer/vendor/laravel/valet/cli/Valet/Status.php on line 181
  1. Try out valet share; you'll now be prompted to install ngrok via Homebrew, or you can try out using it with Expose; I'd love for folks to test both out!
    I have installed ngrok previously using a method other than homebrew, valet has no idea and is trying to run a homebrew version of ngrok which I don't have installed:
$ which ngrok
/usr/local/bin/ngrok
$ valet share
sudo: /opt/homebrew/bin/ngrok: command not found
  1. If you use .valetphprc, rename the .valetphprc file to .valetrc, and prepend php= to your PHP formula; e.g. [email protected]... and test to make sure everything still works!
    I do not use any rc or config files.
  2. If you have local sites on PHP 7.1-7.4, switch your linked PHP version to 8+, install Valet v4, then switch your linked version back to your older PHP (valet use [email protected]) and test to make sure everything still works correctly
    Yep, I have some sites running php 7.4 in isolation and they are still working
  3. If you have any custom local drivers, let me know if you have any trouble getting them working with v4.
    I compared my local drivers with the sample driver that was updated but did not see that a namespace was needed, the error message "Please update your valet drivers" was not really helpful in that regard, you might want to link to the upgrade doc or output a summary of the changes that have to be made. After updating them correctly it worked like a charm.

PHP monitor also still works great.

Appreciate all the hard work!

@nicoverbruggen
Copy link
Contributor

OK, I gave the update a spin!

Installation and general use seems to work fine. Sites are properly served, and I haven't bumped into any major issues. Site isolation also works correctly, but I haven't been able to test with legacy PHP versions yet. (This is definitely something I will test later this week, and if I bump into issues I'll add another comment here.)

The new valet status command works as expected here. I'm running the latest version of Homebrew (4.x) which released recently, so no issues with that.

I don't have any .valetrc files or custom drivers in use so nothing to report on that!


I did run into an issue related to using ngrok. I hadn't used ngrok on this machine yet, so here's what happened:

Upon running valet share, Valet correctly prompted me to install ngrok. Perhaps you can change the text to Would you like to install ngrok now? [Y/n]? I wasn't sure if I simply had to press enter or do something else.

Upon running valet share I got an error that told me to register and set up an ngrok account: "An account is required to use host header rewrite".

It may be worth it to run ngrok config check and check if that is valid before actually starting ngrok?

You could then specify that the user still needs to run ngrok config add-authtoken before they can continue (and point them at dashboard.ngrok.com if the config is invalid or needs to be upgraded) .

After adding the token, sharing seemed to work... but something seems wrong about the port configuration, I'm seeing the same issue that @dcblogdev reported above when I tried to share my local domain phpmon.test (a linked site):
Screenshot 2023-02-21 at 17 26 25

My site is accessible via HTTP (non-TLS/https) on phpmon.test:60... it appears as if the DNS resolution might be the issue, since I do have a custom DNS (1.1.1.1) set in System Settings, but I get the same issue even if I disable it.

This may be a local network thing on my end? Not sure. I was not running any VPN or firewall at the time of testing.

@ashleyshenton
Copy link
Contributor

I have no issues with the installation and all sites are working correctly, but I do have the same ngrok issues as above. I tried expose and this worked without issue.

image

@mattstauffer
Copy link
Collaborator Author

mattstauffer commented Feb 22, 2023

Hey @caseysoftware, is there any chance you can take a look at the Ngrok issues folks are seeing here with ERR_NGROK_8012?

I'll work on adding the [Y/n] prompt (thanks for the suggestion!) and also trying to make sure Valet better handles if you already have ngrok installed on your own. I'll also look into the ngrok config check--thanks @nicoverbruggen!

@laytan I'll see if I can figure what would cause your valet status issues as well.

Thanks y'all!

@mattstauffer
Copy link
Collaborator Author

Hi Matt and Team! I love to use valet + takeout

can you make valet 4 support use shivammatur/php ? So I can use something like this with valet:

valet use shivammathur/php/[email protected]

@caturandi-labs I'm sorry, I don't have any plans to extend valet use to allow passing custom formulas at the moment.

@mattstauffer
Copy link
Collaborator Author

mattstauffer commented Feb 22, 2023

@mattstauffer
Copy link
Collaborator Author

Everyone here: Do you have any outstanding concerns before I release Valet v4 to the public?

@ashleyshenton
Copy link
Contributor

No, I've been using it without issue for a few weeks now. Might just be worth adding a note to the docs about altering the DNS for valet share with ngrok. Nice work on the release @mattstauffer!

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.