Move to Laravel 6

[LukeTowers]

https://www.bountysource.com/issues/75392602-move-to-laravel-6

Laravel 6 has been released and is a LTS release (https://laravel.com/docs/6.x/releases#laravel-6). As such, October should look at moving from 5.5 to 6. This issue will contain discussions relevant to that change and related issues.

Upgrade Guides:

• 5.5 -> 5.6: https://laravel.com/docs/5.6/upgrade
• 5.6 -> 5.7: https://laravel.com/docs/5.7/upgrade
• 5.7 -> 5.8: https://laravel.com/docs/5.8/upgrade
• 5.8 -> 6.x: https://laravel.com/docs/6.x/upgrade

Related Issues: (so far)

• Remove jenssegers/data and move for Carbon 2 #4182 (No longer necessary)
• JSMin minifier is non-free (suggest JSqueeze or JShrink as alternatives) owncloud/core#13052 & https://www.happyassassin.net/2014/12/29/adventures-in-php-web-asset-minimization/ (Change from JSMin to JSqueeze) - (implemented in 627c0aa)
• PHP 7.4 support #4797 will be moving to a fork of Assetic maintained by the core October CMS team, also will need to ensure that we use wikimedia's fork of LessPHP which will require PHP 7.2.9 (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• [PostgreSQL 10] Creating database table rainlab/builder-plugin#212 (comment) (Upgrade doctrine/dbal) (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• Move autoloading to index to make it same with laravel 5.5 #4280 (Update autoloading) (Implemented in e099b7b)
• Replace old selenium functional testing for the new Laravel Dusk #3051 (Laravel Dusk replacing Selenium) (Fixed by [Laravel 6] Add support for Laravel Dusk tests #4919)
• Update PhpUnit version in composer.json #3532 (Update PHPUnit) (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• Missing relatioship type 'HasOneThrough' #4649 (HasOneThrough relationship) (Implemented in octobercms/library@90718d8, unit tests 26944a5)
• SameSite Cookie not working #3893 (SameSite cookie not working) (Fixed by build 462)
• Laravel Authenticatable #3227 (Bring October's Auth system more in line with Laravel's) (This will be investigated for inclusion at a later stage - at the moment, our authentication process is largely incompatible with Laravel's Auth system)
• Method diffInRealSeconds does not exist. #4184 (Update Carbon to Carbon2)
• octobercms/library@c8e2740 (Revisit cookie serialization and encryption serialization in general)
• [Laravel 6] Bring code to a state where unit tests pass library#443 (Unit testing upgrading) (Merged)
• L6 also includes a switch to the Ignition error page handler (https://github.com/facade/ignition), we should look into integrating that into October (We will not be including this in October at this stage.)
• Support added back for serializable closures, also check if route caching works with closures (Queued closures were deprecated in L5.3 docs#425)
• Update marketplace's list of pre-installed composer dependencies: Integrate existing dependencies into Marketplace plugin build process #2831
• October's Translation service doesn't match Laravel's October's Translation service doesn't match Laravel's #5092, fixed by octobercms/library@b8e400b

Breaking changes: (so far)

• storage/framework/cache/data directory needs to be added and !data/ added to the .gitignore file under storage/framework/cache (Fixed by 25511bf)
• Cms\Classes\CmsObjectCollection->where() method signature changed
• Require PHP 7.2 (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• Require PHP Unit 8.0 (unit tests will need to have return type hints added) - also remove 72f85eb when updating (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• Cache TTL has been changed from minutes to seconds when using integers, recommend using datetime objects instead
• May need to swap https://github.com/meyfa/phpunit-assert-gd for https://github.com/lupka/phpunit-compare-images as assertGD doesn't support PHPUnit 7.0 (No longer required, phpunit-assert-gd supports PHPUnit 8 now) (Fixed by Allow support for PHPUnit 8.x meyfa/phpunit-assert-gd#2)
• setUp() and tearDown() methods in unit tests require adding : void as a return type hint to their definitions. (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• If you're utilizing the core unit tests you will need to grab the latest copies of the base unit tests from octobercms/october on GitHub and drop them into your project
• Carbon will be updated from v1 to v2, see https://carbon.nesbot.com/docs/#api-carbon-2 (Still need to go through and upgrade in core)
• Sparkpost & Mandrill mail drivers removed from core Laravel, we should probably add them back to October (i.e. revert laravel/framework@e2461f1#diff-c1d5bf1c77457840314f00f3837a2b2a in octobercms/library) (Implemented in octobercms/library@c8e34ea)
• The Input facade has been removed as it was mostly a mirror of the Request facade, we need to add it back in (https://laravel.com/docs/6.x/upgrade#the-input-facade) (Fixed by [Laravel 6] Bring code to a state where unit tests pass library#443)
• The default redis client has changed from predis to phpredis. Configuration change may be required by users wishing to retain predis, or potentially we need to dynamically set the configuration for people (by adding a client config value defaulting to phpredis and doing Config::set() if the value isn't detected). See more here: https://laravel.com/docs/6.x/upgrade#redis-default-client (Fixed by octobercms/library@4aeaf54, 0394494...a2966da)
• Rackspace storage support was removed. Potentially needs to be re-added? (Re-added in octobercms/library@3ee82fd)
• email validation rule changed internal behaviour to use a broken version of RFC validation instead of PHP's built in validation, need to change it back. See Laravel 6: Email validation filter is too permissive #5070
• trans() and transChoice() methods have been removed from the Translation service, fixed by octobercms/library@a364d64

Other tasks:

• Integrate old helpers (https://github.com/laravel/helpers/blob/master/src/helpers.php) into Rain library. (Implemented in octobercms/library@af29bbf)
• Update the Unit Testing documentation in the docs repo and https://github.com/octobercms/october/tree/wip/laravel-6/tests#plugin-testing
• Confirm Selenium tests in tests/functional directory still work, update README if necessary. (Replaced with Dusk testing: [Laravel 6] Add support for Laravel Dusk tests #4919)
• [Low priority] Add Selenium tests to GitHub Actions workflow.
• Go through all of the documentation under octobercms/docs that originally mirrored the Laravel docs and update it to reflect the latest changes to the Laravel docs.
• Update Validator to include latest rules. (fixed by 9911c6b)
• Update Laravel Tinker to 2.0 to improve compatibility of dependencies octobercms/library@6fae9cd

Known issues:

• Signed URLs do not seem to work. (fixed by octobercms/library@c258e66)
• php artisan cache:clear removes the required storage/framework/cache/data folder.

[danharrin]

Will you be moving to the Laravel 5.7 email verification system?

[LukeTowers]

@danharrin what's that?

[danharrin]

@LukeTowers https://laravel.com/docs/master/verification, built-in email verification into the Laravel auth system

[LukeTowers]

@danharrin Probably not? October still isn't using Laravel's auth system so that sounds like a headache to implement.

[bennothommo]

Another related issue:
rainlab/builder-plugin#212 (comment) (Laravel 5.9 should include a compatible version of doctrine/dbal)

[bennothommo]

Just to note too, Laravel 5.9 is going to actually require at least PHP 7.2, according to their composer.json.

[adrenth]

Finally time to implement strict types! And in all the DocBlocks too.

[Samuell1]

Hope this gets merged too :D #4280

[ghost]

Is it possible to turn October CMS into an "ever-green" and just have rolling updates that follow Laravel, so let's say we are on stable 5.8 laravel, then October is on 5.7 laravel etc. When 5.9 laravel comes out we are on 5.8 etc. that way October just's updates all the time and is one version down so they can sort out the bug fixes in the meantime.

There's really big LTS release gaps.

See here timeline: https://laravel-news.com/laravel-release-process

Seems like every 2-3 years. In that time so much happens. 5.5 jump to 5.9

I remember 3 years ago we had a vote to update to LTS 5.5 can't we just have another vote to get rolling updates. Every release offers many valuable things! I remember Laravel 5.7: New Pagination Link Customizations and getting excited about that feature and then disappointed I couldn't use them in October.

[ghost]

Also maybe updating to Laravel 5.9 will fix this issue as well: #3893

[LukeTowers]

@ayumihamsaki no, not unless someone is willing to pay for that work to be done. Just to give you some background on how we feel about Laravel's breaking changes in every release, we've toyed around with the idea of moving completely away from Laravel sometimes. Not saying that's going to happen anytime soon but it's incredibly unlikely that we'd ever decide to stay up to date with the latest Laravel changes for every single release.

[adrenth]

That's why they invented semver

[Samuell1]

@adrenth hope octobercms will be using that with new release because now only patch version is changing but with breaking changes or new features minor version should change too and major should change when laravel core will be updated. What you think @LukeTowers ?

[ghost]

Cool thanks for answers.

[LukeTowers]

Semver would help with a lot of issues but that's up to @daftspunk and I don't think he's a fan. We also don't really want to ever have to support multiple versions of October, we try our best to minimize the breaking changes that come from Laravel as much as possible to encourage people to remain up to date with October.

[Denoder]

@LukeTowers dont forget Carbon, now Laravel gives you an annoying message every time you update to use Carbon 2 instead of Carbon 1. What other framework were you looking at as a possibility if you were to move away from Laravel? Because I get where you're coming from.

[adrenth]

Move to another framework is not even remotely possible. So that would be out of the discussion. That would result in a total new product, which will throw years of work away.

My suggestion would be to build a stable version of OC with the next LTS version of Laravel and introduce semver so vendor packages and OC itself can be locked in a state everyone is familiar with nowadays.

[Denoder]

I'm not saying to move away, im asking what was the other option he was considering.

I think #3683 needs to be resolved first before trying to add more workload by branching off.

[LukeTowers]

@adrenth @teranode the idea that's been talked about lots by @daftspunk is basically forking Laravel and maintaining that. Personally, I'm not a fan of doing that as it would diverge from the Laravel community and limit our potential reach, so I don't see us doing that any time soon, if at all. I bring it up purely to communicate our frustration with their often nonsensical breaking changes.

@teranode is there a good resource for any breaking changes from switching to Carbon 2?

[Denoder]

@LukeTowers
wew that idea sounds worse than just not using Laravel at all, but I do get that frustration as i tend to look at their codebase every now and then and see things that don't need to be there.

Nesbot got a documentation for their breaking changes:
https://carbon.nesbot.com/docs/#api-carbon-2

[nathan-van-der-werf]

wew that idea sounds worse than just not using Laravel at all, but I do get that frustration as i tend to look at their codebase every now and then and see things that don't need to be there.

You don't have this frustration when looking at the OctoberCMS codebase?

[LukeTowers]

@nathan-van-der-werf anything in particular? 😉

[nathan-van-der-werf]

@nathan-van-der-werf anything in particular? 😉

My biggest frustration (aside from not having semver and tags being made while the version is not "stable" yet) would be the lack of strict typing.

[LukeTowers]

You mean not setting strict type mode at the top of every file?

[nathan-van-der-werf]

You mean not setting strict type mode at the top of every file?

The code not being strict at all, some examples:

• https://github.com/octobercms/october/blob/master/modules/system/classes/PluginBase.php#L93

• https://github.com/octobercms/october/blob/master/modules/system/classes/PluginBase.php#L83

• https://github.com/octobercms/october/blob/master/modules/system/classes/PluginBase.php#L43
  No strict typing, while it should always return an array

• https://github.com/octobercms/october/blob/master/modules/system/classes/PluginBase.php#L153
  $schedule is not a string here... It's an object (not strict)

• https://github.com/octobercms/october/blob/master/modules/system/classes/PluginBase.php#L260
  Why aren't the $key and $class strictly typed to string when it has to be a string

(*and this is only 1 of the who knows how many files not being strict)

[Denoder]

some of that seems like nitpicky stuff someone could PR in if they were concerned about it.

[multiwebinc]

In response to my previous comment, I looked into the abandoned package warnings I'm getting when running composer update:

Package jakub-onderka/php-console-color is abandoned, you should avoid using it. Use php-parallel-lint/php-console-color instead.
Package jakub-onderka/php-console-highlighter is abandoned, you should avoid using it. Use php-parallel-lint/php-console-highlighter instead.

These are caused by laravel/tinker which requires an old version of psy/psysh, which requires jakub-onderka/php-console-highlighter. This was fixed by psy/psysh in the following commit:

bobthecow/psysh@2a75895

So basically the only way to remove these warnings would be to use a newer version of tinker.

[LukeTowers]

@multiwebinc so they don't use that version in Laravel 6?

[multiwebinc]

@LukeTowers As of Laravel 6.8.0, Tinker was updated from v1 to v2. See: laravel/laravel#5161

[multiwebinc]

@LukeTowers I'm guessing this could be updated in https://github.com/octobercms/library/blob/wip/laravel-6/composer.json ?

[LukeTowers]

It could be indeed, feel free to submit a PR to the L6 branch with the update. Just checked their changelog for 2.0 and it looks like it was just dependency updates that are all compatible with our 6.x changes so it should be fine to just merge it in no problems.

[multiwebinc]

@LukeTowers Sounds good. octobercms/library#494

[patosG]

I'm trying to install L6 branch i have after a composer udate

> php artisan october:util set build

In ServiceProvider.php line 128:
                                                                               
  Call to undefined method October\Rain\Translation\Translator::addJsonPath()

Am I the only one ?

[LukeTowers]

@shiftaltd can you report that in a separate issue along with the full stack trace (if you can get it) and the exact steps you took? I haven't encountered that issue myself and I've moved 10-20 production sites over to the L6 branch without any issues.

[patosG]

@LukeTowers ok. there is probably something with my not so fresh install. I'll do a fresh one on a new vps to test. Thanks

[patosG]

Sorry. It works. my error was due to laravel\cashier in composer.json. I have to find the L6 cashier version i guess

[LarryBarker]

I'm not sure if this is related to the move to L6 or not, but I am having a tough time updating my dependencies. I have been working with @LukeTowers to troubleshoot, and have yet to resolve the issue. I have removed modules, vendor, and all of my plugins, and running composer update is throwing the following error:

post-update-cmd: php artisan october:util set build
Executing command (CWD): php artisan october:util set build

In Container.php line 805:
                                        
  Target class [files] does not exist.  
                                        

In Container.php line 803:
                              
  Class files does not exist  
                              

Script php artisan october:util set build handling the post-update-cmd event returned with error code 1

I also cannot get the stack trace from the browser because I am getting a 500 error using Valet. Has anyone else had a similar issue?

UPDATE
Ok I was finally able to narrow this down to barryvdh/laravel-ide-helper. Still not sure what is causing it, so I'll investigate more and report back here.

[LukeTowers]

@LarryBarker I just tested adding that package as a requirement to a project running the L6 branch and had no problems, so not sure what's going on there for you. I did however run into that annoying issue with the invalid laravel cache files for the service providers / aliases after removing the package, and I think I've come up with a solution here: octobercms/library#492 (comment)

[freddy38510]

@LarryBarker
I faced the same issue than you.
I found why the class "files" could not be found in this pull request .

So, i forked laravel-ide-helper and made a commit to fix the issue.

I'm using the fork like this in the composer.json file of my own plugin:

   "repositories": [
        {
            "type": "git",
            "url": "https://github.com/freddy38510/laravel-ide-helper/"
        }
    ],
    "require": {
        "composer/installers": "~1.0"
    },
    "require-dev": {
        "barryvdh/laravel-ide-helper": "master"
    }

Don't forget to set merge-dev to true in your root composer.json.

   "extra": {
        "merge-plugin": {
            "include": [
                "plugins/*/*/composer.json"
            ],
            "recurse": true,
            "replace": false,
            "merge-dev": true
        }
    }

[ericp-mrel]

I was looking through the route caching item and testing to see if they worked with closures.
I noticed all of the route registrations happen inside of the App::before() calls and I was curious what the original reason for that was. As it stands with the routes for the different modules being registered in that way it causes the route:cache command to fail since it thinks there aren't any routes. However, if you remove the App::before() and just register the routes directly through the routes.php files, all of the routes are then able to be listed in the route:list command and also allows the routes to be cached as well.

This seems like an easy solution, however I just wanted to know what the original purpose of the routes being registered that was for and what issue it was solving since I imagine there was a reason for doing it that way.

[LukeTowers]

@ericp-mrel it has to do with when the plugin routes get registered vs the module routes vs any routes added by third party packages. We had an internal discussion about it a few weeks ago and I'll be making some changes to it in the next month or so. Note that any plugin routes are still listed and cached just fine, it's just the module ones that aren't, which isn't really that big of a deal because there's like 6 total.

[alireza-salehi]

can we use "wip/laravel-6" branch as a clean installation for new projects or is it too soon?

[LarryBarker]

can we use "wip/laravel-6" branch as a clean installation for new projects or is it too soon?

@alireza-salehi I’m using this branch in a production environment. Just make sure you follow the upgrade guide completely.

[Samuell1]

@LukeTowers even plugin routes arent cached too because they going trough main routing not trough ocms custom.

[LukeTowers]

@Samuell1 ?

[Samuell1]

@LukeTowers forgot to mention iam responding to this msg, and i if you run route:cache it doesnt cache anything because of that issue with initializing of routes.

Note that any plugin routes are still listed and cached just fine, it's just the module ones that aren't, which isn't really that big of a deal because there's like 6 total.