Implemented Warning when building addon with composer packages not installed

[mattrogowski]

If you try and install an addon on the CLI but haven't installed the composer packages first, you get an error The site is currently being upgraded. Please check back later. repeated several times. As it wasn't obvious this was due to missing composer packages I disabled listeners and installed the addon and then built a release.

If you have an addon installed and try and build a release with listeners enabled and haven't installed the packages, you get an error [InvalidArgumentException]Composer Autoload path (src/addons/.../vendor/composer) does not appear to be a valid composer directory, as I guess it runs some of the addon code. However with listeners disabled it just installs and builds.

I expect that disabling listeners is not the best solution to the original error I had, however I think it'd still be helpful to explicitly check vendor/composer exists before doing the build instead of an exception being thrown when it can't load it. Or, maybe better, check vendor/composer exists before you try and install or upgrade the addon. The "The site is currently being upgraded" error isn't very clear, if it output an error saying something like "Missing composer_autoload directory" or something, would be clear that needs to happen first.

tl;dr when trying to install an addon or building a release without installing the packages, have a nice handled error about the missing composer directory so it's clear that needs installing.

This is only a developer-centric issue but could probably be handled a bit better than it is.

 

[Xon]

I would personally make this a bug report as reporting The site is currently being upgraded. Please check back later when trying todo an add-on build is rather misleading and a pain to recover from.

 

[Sim]

Having experienced this myself, I covered this issue in my Using Composer Packages in XenForo 2.1+ Addons Tutorial

Bug reports notwithstanding, I suggest you mitigate against this issue by adding some code in the checkRequirements function in your Setup.php file.

I have the following code (in the correct namespace) in all of my addons that use composer packages:

<?php namespace ComposerTutorial;

use XF\AddOn\AbstractSetup;
use XF\Db\Schema\Create;

class Setup extends AbstractSetup
{
    public function install(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function upgrade(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function uninstall(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function checkRequirements(&$errors = [], &$warnings = [])
    {
        $vendorDirectory = sprintf("%s/vendor", $this->addOn->getAddOnDirectory());
        if (!file_exists($vendorDirectory))
        {
            $errors[] = "vendor folder does not exist - cannot proceed with addon install";
        }
    }
}

 

[Xon]

Having experienced this myself, I covered this issue in my Using Composer Packages in XenForo 2.1+ Addons Tutorial

Bug reports notwithstanding, I suggest you mitigate against this issue by adding some code in the checkRequirements function in your Setup.php file.

I have the following code (in the correct namespace) in all of my addons that use composer packages:

<?php namespace ComposerTutorial;

use XF\AddOn\AbstractSetup;
use XF\Db\Schema\Create;

class Setup extends AbstractSetup
{
    public function install(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function upgrade(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function uninstall(array $stepParams = [])
    {
        // Nothing to do yet
    }

    public function checkRequirements(&$errors = [], &$warnings = [])
    {
        $vendorDirectory = sprintf("%s/vendor", $this->addOn->getAddOnDirectory());
        if (!file_exists($vendorDirectory))
        {
            $errors[] = "vendor folder does not exist - cannot proceed with addon install";
        }
    }
}

This is a better check;

public function checkRequirements(&$errors = [], &$warnings = [])
{
    $json = $this->addOn->getJson();
    $composerPath = $json['composer_autoload'] ?? '';
    if (\strlen($composerPath))
    {
        $vendorDirectory = $this->addOn->getAddOnDirectory() . \XF::$DS . $composerPath;
        if (!\file_exists($vendorDirectory))
        {
            $errors[] = "vendor folder does not exist - cannot proceed with addon install";
        }
    }
}

I'm going to add this to my StandardLib add-on

 

[Chris D]

I believe we've actually mitigated this already in XF 2.2.6 due to the changes we made in this bug report:

Fixed - Add-on Composer support seems inefficient

If an Add-on does have a composer autoloader directory defined, XenForo will check if that directory and installed.json within that path does exist in XF\ComposerAutoload::__csonstruct() if (!file_exists($pathPrefix) || !file_exists($pathPrefix . \XF::$DS . 'installed.json')) { throw new...

xenforo.com

This has changed the error handling in such a way that you can't actually install or upgrade an add-on any longer that has a missing composer directory that has been defined.

When installing from an archive, the install gracefully fails and is reported on the results page.
When installing from the CLI or the Admin CP when the files have been uploaded, an exception is thrown.

That applies with debug mode enabled. If you don't have debug mode, the failures are silent, errors are logged, and the composer autoload isn't set up.

This will obviously result in other errors if the code calls unloaded packages so we need to make sure there's a build-time check for the composer path too so consider that done for the next release as well.

 

[mattrogowski]

That sounds great, thank you! Anything that makes it a bit harder to miss installing them is a plus.