Drupal-Check, Site Factory, and Acquia BLT, OH MY!

The Backstory

I was fortunate enough to attend DrupalCon Seattle this year, as well as give a presentation on mental health in tech, but one of the key topics of DrupalCon was Drupal 9 readiness. Dries mentioned it several times in the Driesnote and we even had some contribution efforts specific to Drupal 9 readiness at MidCamp 2019.

One thing that stuck out to me as particularly awesome was my friend and fellow MidCamp organizer Matt Glaman's drupal-check tool that will check your modules for deprecated code that is NOT ready for Drupal 9 being mentioned in the Driesnote. Additionally just as awesome was another good friend, Dwayne McDaniel getting a shoutout for going through EVERY. SINGLE. CONTRIB. MODULE. and running this tool against it. That's a lot of modules!

In case you didn't know, the upgrade path from Drupal 8 to Drupal 9 is supposed to be extremely easy. As someone who had to migrate from 7 to 8, this is welcome news. The TL;DR version is: Once there are backwards compatibility breaks, we're at a new version. What does this mean? It means that code that is marked as deprecated in 8 will NOT be in 9 and you can't upgrade from 8 to 9 if you're using this deprecated code. (I'm looking at you array())

The Problem(s)

Drupal Check is AMAZING! That's not a problem. The first problem is making sure that your code is ready to be checked by Drupal Check.

Issue 1 (Acquia Cloud Site Factory/Multisite)

We use Acquia Cloud Site Factory extensively at Genuine. This leads to many multi-site installs. Why is this an issue? Well, it's not unusual to have multiple profiles available on Site Factory. Often times, these profiles will be based off another one, but different in theme, look and feel, or functionality. This means that there is a chance that there will be some duplicated functions in the .theme file. Specifically, helper functions that may not fall into the _THEMENAME_function_name() convention and may be missed when you change everything around.

Fatal error: Cannot redeclare _my_poorly_named_function() (previously declared in /var/www/docroot/profiles/custom/profile_1/themes/profile_1_theme/profile_1_theme.theme:150) in /var/www/docroot/profiles/custom/profile_2/themes/profile_2_theme/profile_2_theme.theme on line 168

It happens. Stop looking at me like that.

Solution 1

Well, this is a pretty easy fix, again compliments of Matt Glaman and Will Long AKA Kerasai. You can either prefix the functions OR use namespaces!

<?php

namespace my_profile_name;

Your .profile files are autoloaded so the namespace will be valid. This prevents the error above.

Issue 2 (Acquia BLT)

This one is a doozy. Acquia BLT or Build and Launch Tool (as of 9.2) has a little command in it that checks for deprecated code, but the dependencies it uses are outdated and cause Drupal Check to fail before it gets anywhere.

blt tests:php:sniff:deprecated

This command depends on the package sensiolabs-de/deprecation-checker which has a dependency on nikic/php-parser:~3.0. Drupal Check requires nikic/php-parser:^4.0 due to the way it's built. Trying to run composer require nikic/php-parser:^4.0 will make composer yell at you. A lot.

Solution 2

This one gets a bit tricky. There are a few things that need to happen here if you're wanting to reap the benefits of Drupal Check, and trust me, you do.

In your base composer.json file there should be a couple of lines that look like this

        "merge-plugin": {
            "require": [
                "blt/composer.required.json"
            ],
            "include": [
                "blt/composer.overrides.json"
            ],

The composer.required.json file is in your blt/ folder and is autogenerated by BLT so it's not safe to change. The workaround for this is to copy the file to composer.required-modified.json and update the line below "require": [ to blt/composer.required-modified.json. The end result should look like this:

"merge-plugin": {
            "require": [
                "blt/composer.required-modified.json"
            ],
            "include": [
                "blt/composer.overrides.json"
            ],

Next, you will need to run the command composer remove sensiolabs-de/deprecation-detector This should remove the old package and update nikic/php-parser but if it doesn't, check in your new composer.required-modified.json file, make sure that Deprecation Detector is gone, and add in "nikic/php-parser": "^4.0" at the end of the "require-dev" section. DON'T FORGET YOUR COMMA!

Now you should be able to run Drupal Check without issues, but there's still one thing we need to take care of before saying we're done. That little BLT command from above blt tests:php:sniff:deprecated. If we try running it without the deprecation-detector package then we're going to have a bad time.

My workaround for this was to create a replacement command for BLT. This is done by using replace command annotation in a custom class. The BLT docs are here but they are lacking and caused me some headaches, so I've got some docs for you right here.

My custom command file lives in blt/src/Hooks/NoDeprecatedCommandHook.php. The name of the file and class needs two things:

  1. It can't be the same as the class you're replacing.
  2. It needs the word Hook at the end.

DeprecatedCommandHook.php will not work. NoDeprecatedCommand.php will not work. MyReplacementCommandHook.php will definitely work, so long as your class shares the same name, but let's stick with my own file here.

<?php

namespace Acquia\Blt\Custom\Hooks;

use Acquia\Blt\Robo\BltTasks;

/**
 * Defines commands in the "tests:php:sniff:deprecated*" namespace.
 */
class NoDeprecatedCommandHook extends BltTasks {

  /**
   * Detects usage of deprecated custom code.
   *
   * @hook replace-command tests:php:sniff:deprecated
   *
   * @aliases tpsd deprecated
   */
  public function detect() {
    $this->say("This command has itself been deprecated. Please use drupal-check for all of your deprecated code needs.");

    return 0;

  }

}

Let's walk through the file a bit

namespace Acquia\Blt\Custom\Hooks;

Without this line, it won't work It needs to be in this namespace or you're going to have a bad time.

use Acquia\Blt\Robo\BltTasks;

This is optional, but I used it to get the say() method available.

class NoDeprecatedCommandHook extends BltTasks {

Our class name. If you're using BltTasks, don't forget to extend it.

  /**
   * Detects usage of deprecated custom code.
   *
   * @hook replace-command tests:php:sniff:deprecated
   *
   * @aliases tpsd deprecated
   */

This is where the magic happens. @hook replace-command tells the system to forget about the original command, this one is driving now.

  public function detect() {
    $this->say("This command has itself been deprecated. Please use drupal-check for all of your deprecated code needs.");

    return 0;

  }

This is our function that does nothing more than tell us to stop using this function. Clever, right?

The end result is a functioning drupal-check and a function that tells its own deprecation.

➜  project git:(master) ✗ blt tests:php:sniff:deprecated
This command has itself been deprecated. Please use drupal-check for all of your deprecated code needs.

Conclusion

This is a pretty specific use case and hopefully Acquia BLT will be pushing out an update in the near future that doesn't require us to have to workaround much longer, but this works for us.

Please test the hell out of your project after making these updates. BLT should really only be used for local dev and deployment artifacts, but it's still worth noting what does and doesn't happen that might bork up your site.

Edit: BLT is going to be getting rid of the DeprecatedCommand function that is replaced in this post in the very near future from 5/8/19. That will help all of you out, but it also means that I spent way too much time typing this thing out. See: https://github.com/acquia/blt/pull/3621

Category