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

Fix: Correct vendor directory path resolution for Laravel Artisan Tin… #179

Closed
wants to merge 1 commit into from

Conversation

hmjubayed
Copy link

Overview

This pull request addresses an issue with the Laravel Artisan Tinker command where the vendor directory path might not be correctly determined when developers customize the vendor directory. This fix ensures flexibility in determining the vendor directory path while resolving issues with running the Laravel Artisan Tinker command in various environments.

Changes

  • Updated the code to dynamically determine the default vendor directory path based on the Laravel base path.
  • Added a check to ensure the determined vendor directory is valid.
  • If the vendor directory is not valid, it falls back to using the path obtained from InstalledVersions::getRootPackage().
  • Updated the code to use Env::get to retrieve the COMPOSER_VENDOR_DIR environment variable, allowing developers to customize the vendor directory path.

Detailed Changes

use Composer\InstalledVersions;
$vendorDir = $this->getLaravel()->basePath().DIRECTORY_SEPARATOR.'vendor';
if (!is_dir($vendorDir)) {
    $vendorDir = realpath(InstalledVersions::getRootPackage()['install_path']) . DIRECTORY_SEPARATOR . 'vendor';
}

$path = Env::get('COMPOSER_VENDOR_DIR', $vendorDir);

Testing

Verified that the php artisan tinker command works correctly with and without the COMPOSER_VENDOR_DIR environment variable set.
Ensured that the fallback to InstalledVersions::getRootPackage() is correctly handled when the vendor directory does not exist.

Impact

This fix ensures that developers have the flexibility to customize the vendor directory path while maintaining the stability of the Laravel Artisan Tinker command in various deployment environments.

Please review the changes and provide feedback. This revision emphasizes the importance of flexibility in determining the vendor directory path, accommodating developers who may customize this directory according to their needs.

@taylorotwell
Copy link
Member

I am closing this pull request because it lacks sufficient explanation, tests, or both. It is difficult for us to merge pull requests without these things because the change may introduce breaking changes to the framework.

Feel free to re-submit your change with a thorough explanation of the feature and tests - integration tests are preferred over unit tests. Please include it's benefit to end users; the reasons it does not break any existing features; how it makes building web applications easier, etc.

Thanks!

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.

2 participants