laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.52k stars 11.02k forks source link

[11.x] After Laravel 11.24.0 commands in `app/Console/Commands` are not auto-registered #52902

Closed rodrigopedra closed 1 month ago

rodrigopedra commented 1 month ago

Laravel Version

11.24.0

PHP Version

8.2.23

Database Driver & Version

mysql Ver 8.0.36 for Linux on x86_64 (MySQL Community Server - GPL)

Description

PR #52867 introduced a change to verify if the routes/console.php file exists. After this change, the commands in app/Console/Commands are not auto-registered anymore, if both the routes/console.php, and commands in app/Console/Commands exist.

Steps To Reproduce

Public repo:

https://github.com/rodrigopedra/bug-report-laravel-11-24

Instructions are in the README

Previously, I was manually registering the routes/console.php file

rodrigopedra commented 1 month ago

One solution, is to always auto-register commands in app/Console/Commands:

public function withCommands(array $commands = [])
{
    if (empty($commands) && is_file($this->app->basePath('routes/console.php'))) {
        $commands = [...$commands, $this->app->basePath('routes/console.php')];
    }

    // CHANGED: remove the empty() check in this if clause
    if (is_dir($this->app->path('Console/Commands'))) {
        $commands = [...$commands, $this->app->path('Console/Commands')];
    }

    $this->app->afterResolving(ConsoleKernel::class, function ($kernel) use ($commands) {
        [$commands, $paths] = collect($commands)->partition(fn ($command) => class_exists($command));
        [$routes, $paths] = $paths->partition(fn ($path) => is_file($path));

        $this->app->booted(static function () use ($kernel, $commands, $paths, $routes) {
            $kernel->addCommands($commands->all());
            $kernel->addCommandPaths($paths->all());
            $kernel->addCommandRoutePaths($routes->all());
        });
    });

    return $this;
}
rodrigopedra commented 1 month ago

ping @taylorotwell @driesvints

SamuelNitsche commented 1 month ago

@rodrigopedra Sorry, that's my bad. I created a PR to fix this.

rodrigopedra commented 1 month ago

Don't worry =)

No production servers were harmed. At least not on my deployment =)

I rushed to let them know as I imagine it could break some people's apps that depend on scheduled commands, or have a crontab calling any custom command directly

NightingaleWK commented 1 month ago

After the release of the new version, I found that my Commands were invalid, I wrote them all in App\Console\Commands, and it was all such a coincidence

My own temporary solution: register commands by providing the command's class name to the withCommands method

<?php

use App\Console\Commands\CalculateFee;
use App\Console\Commands\DropAllTables;
use App\Console\Commands\PayFee;
use App\Console\Commands\RefreshYearAnalyse;
use Illuminate\Foundation\Application;
use Illuminate\Foundation\Configuration\Exceptions;
use Illuminate\Foundation\Configuration\Middleware;

return Application::configure(basePath: dirname(__DIR__))
    ->withRouting(
        web: __DIR__ . '/../routes/web.php',
        commands: __DIR__ . '/../routes/console.php',
        health: '/up',
    )
    ->withMiddleware(function (Middleware $middleware) {
        //
    })
    ->withExceptions(function (Exceptions $exceptions) {
        //
    })
    ->withCommands([
        CalculateFee::class,
        DropAllTables::class,
        PayFee::class,
        RefreshYearAnalyse::class,
    ])
    ->create();

Fortunately, everything found in the development environment.Wait for specific solutions ^.^

guille1988 commented 1 month ago

My project has also problems with the same thing.

usesorane commented 1 month ago

You can add the following to bootstrap/app.php to re-enable normal behavior until this is fixed.

->withCommands([ __DIR__.'/../app/Console/Commands', ])

SRWieZ commented 1 month ago

Can confirm this :+1:

ziming commented 1 month ago

yup got this issue

kminek commented 1 month ago

@SamuelNitsche shoudn't a fix contain a test case for this bug?

SamuelNitsche commented 1 month ago

@SamuelNitsche shoudn't a fix contain a test case for this bug?

Ideally the whole ApplicationBuilder itself would be covered by tests. However this isn't the case and I unfortunately wasn't sure how to properly test this. I guess Taylor wasn't sure either while developing it.

kminek commented 1 month ago

@SamuelNitsche shoudn't a fix contain a test case for this bug?

Ideally the whole ApplicationBuilder itself would be covered by tests. However this isn't the case and I unfortunately wasn't sure how to properly test this. I guess Taylor wasn't sure either while developing it.

i think this case should be covered with integration test somewhere in https://github.com/laravel/framework/tree/11.x/tests/Integration/Console

tsakib360 commented 1 month ago

Add this in bootstrap/app: ->withCommands([ DIR.'/../routes/console.php', DIR.'/../app/Console/Commands', ])

Airgerr commented 3 weeks ago

So... it was reverted and it is not fixed?? Should this be reopend?

guille1988 commented 3 weeks ago

Yes it was fixed

Airgerr commented 2 weeks ago

Yes it was fixed

I think not: click on the PR, scroll down... read: "this PR has been reverted due to breaking change with existing application." I'm still manually bootstrapping at the moment.

Rafnexx commented 2 weeks ago

Any update on this? I've tried couple of solutions for this problem suggested here but I can't make it work, my commands are still not discovered by Laravel.