-
Notifications
You must be signed in to change notification settings - Fork 718
Allow laravel/valet to be use as dependency inside another project #1539
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
base: master
Are you sure you want to change the base?
Conversation
|
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? |
|
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! |
|
I'm trying to fix the tests in my local machine, but for some reason I'm getting same error, it said Many thanks. |
|
@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? |
|
More digging, commit that cause the problem: |
|
@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 doneValet 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
The "usual" locations for the 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 |
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.