Skip to content

Conversation

@gravataLonga
Copy link

Hello,

I've a few free time on my hand and I was hacking and testing some hidden features inside Laravel Valet, but only work in CLI Enviroment, just by change one line I can call facades methods in Web Enviroment.

Just by change one line I can unlock so much hacking for a new control panel for Valet in my personal computer.

Just asking if any one could read my pull request? If this break any policy, I'm sorry, I did read some links to how to make a pull request.

@mattstauffer mattstauffer changed the title CHG: Allow laravel/valet to be use as dependency inside another project Allow laravel/valet to be use as dependency inside another project Dec 8, 2025
@mattstauffer
Copy link
Collaborator

I'm happy to review it! You've got failing tests in there, so you'll want to see if you can fix those first.

I did quite a bit of work a year or two ago to try to make Valet able to run in another project as a dependency, so I'm 100% on board with that. Is this one minor change all it takes to get it back there?

@gravataLonga
Copy link
Author

Hey @mattstauffer, thanks for taking the time to reply. I was digging through the Git history and it looks like this used to work, but something changed along the way and broke it.

In my tests, adding a condition around that constant let me require it as a dependency in another project. I just defined the constant in my own project to point to Homebrew on my machine.

I’ll fix those tests in the meantime.

Thanks for your patience!

@gravataLonga
Copy link
Author

I'm trying to fix the tests in my local machine, but for some reason I'm getting same error, it said Error: Trait "Illuminate\Support\Traits\ReflectsClosures" not found, which not make any sense if I did run composer install. Probably I'm missing something, could you help me understand what I'm missing?

Many thanks.

@gravataLonga
Copy link
Author

@mattstauffer Sorry for the spam, but it looks like there’s already an issue in master that’s unrelated to the code I worked on: GitHub Actions Log.

Regarding the issues with PHPUnit, I’m not entirely confident in my solution, as it relies on dependencies explicitly declaring a constant to point to their local Homebrew path. Ideally, it would be better if Laravel/Valet could automatically resolve the Homebrew path itself. Do you agree?

@gravataLonga
Copy link
Author

More digging, commit that cause the problem:
laravel/framework@d4d36e8

@gravataLonga
Copy link
Author

@mattstauffer Hey! Sorry for the spammy comments. Could you please take a look at my latest commit and let me know what you think?

What's been done

Valet should resolve all of its dependencies internally rather than relying on external ones, so I created a class to guess the Homebrew path from PHP_BINARY. It might not be the perfect solution, but it works for now.

Why can't the Homebrew path be resolved from the Process class in a web context?

From what I understand, in web context $PATH variable is completely different from CLI context. That's why I'm trying to guess it from PHP_BINARY. I'm not 100% sure this is best approach, though.

As a fallback, the project who is requiring could define a constant, that way we can control some edge cases when needed.

Need more context on how I got this solution?

Check my stream on x.com and will see my struggles. Let me know if you need more clarification or if you need any adjusting to the code.

https://x.com/i/broadcasts/1OdKrOREknlGX

Nonetheless I did create one more test just to test that WebContext class could resolve correctly.

public function test_it_can_guess_correctly_web_context()
{
$files = Mockery::mock(Filesystem::class);
$files->shouldReceive('exists')->once()->with('/not-exits/bin/brew')->andReturn(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor observation: with not-exits did you mean not-exists?

Copy link
Author

Choose a reason for hiding this comment

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

Hello, sorry it was a typo. I meant not-exists

@drbyte
Copy link
Contributor

drbyte commented Dec 13, 2025

The "usual" locations for the brew binary are ['/opt/homebrew', '/usr/local'], but in power-user scenarios someone might locate it elsewhere. Looking in the directory where the php binary is found "might" work, but there are situations (such as Laravel Herd) where the PHP binary is nowhere close to where the brew binary is found (Granted, running Valet at the same time as Herd is probably not recommended IMO.)

Given that Valet's PHP-FPM and Nginx configurations don't pass the BREW_PREFIX environment variable through to PHP, we can't rely on it being present. I suppose that could be added to the default config though, and then FPM would have it.

I would have expected that in "most" cases using exec to shell out to call brew prefix should be reliable if the environment variable can't be read. But clearly you've run into situations where that didn't work, hence your choice to add the WebContext class.

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.

3 participants