cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
508 stars 51 forks source link

v0.3.0 Installing issue (Fatal error) #76

Closed Nobiuss closed 6 years ago

Nobiuss commented 6 years ago

Hi Team,

We have seen the update here on github so we removed the old version and installed this new one and had some errors.

Plugin could not be activated because it triggered a fatal error. ----->

Fatal error: Uncaught LogicException: Function 'SatisPress\autoloader_classmap' not found (function 'SatisPress\autoloader_classmap' not found or invalid function name) in /home//apps/plugins/wp-content/plugins/satispress/satispress.php:48 Stack trace: #0 /home//apps/plugins/wp-content/plugins/satispress/satispress.php(48): spl_autoload_register('SatisPress\auto...') #1 /home//apps/plugins/wp-admin/includes/plugin.php(1897): include('/home/...') #2 /home//apps/plugins/wp-admin/plugins.php(178): plugin_sandbox_scrape('satispress/sati...') #3 {main} thrown in /home//apps/plugins/wp-content/plugins/satispress/satispress.php on line 48

<----

Kind regards, Kim

GaryJones commented 6 years ago

Thanks for reporting, Kim. We'll take a look 👍

thefrosty commented 6 years ago

It looks live version 0.3 requires the plugin to be installed via composer (composer install so the vendor directory is present) as it's not including the functions.php file manually. Is the intended? I was hoping to run this plugin outside of my normal composer sites in some stand-alone installs I could download and upload the zip too.

bradyvercher commented 6 years ago

@thefrosty To upload a zip instead of using Composer, you need to download the packaged release asset that includes the vendor directory. It's available from the releases page and is the file named satispress-{version}.zip. I'm not sure if there's a better way to highlight that or not.

@Nobiuss Does using the release asset fix the error you're seeing?

thefrosty commented 6 years ago

That makes sense to me. I didn't try that, was just looking at the code.

thefrosty commented 6 years ago

Just testing again locally the way I imaging quite a few users might install this plugin; via afragen/github-updater. Which installs the latest master branch. I would suggest adding the custom release-assets header so it knows to download $repo-$tag.zip

Update: Oh, I see that there. Looks like it wasn't honored via the GitHub Updater.

bradyvercher commented 6 years ago

That header is already present, but I've never tried installing a plugin using the updater. Do you know if anything else is required to make it use the release asset for installation?

thefrosty commented 6 years ago

Thanks, I've asked that question: https://github.com/afragen/github-updater/issues/712

afragen commented 6 years ago

Unfortunately Installing from GitHub Updater has no knowledge of release assets or the plugin headers.

As the plugin requires post processing it must either be installed from the release asset link or via composer.

One could immediately edit the main plugin file and decrease the version number after Installing via GitHub Updater with the immediate notice for updating. This update will pull the release asset. Any attempt to activate the plugin prior to this will result in the error above.

GHU simply not set up to install via a release asset URL.

thefrosty commented 6 years ago

I can confirm the the fatal error only happens when installing the plugin via the github-updater or master download from the download link (non-release asset version). When using the release version it has zero issues.

GaryJones commented 6 years ago

@bradyvercher

I'm not sure if there's a better way to highlight that or not.

If someone is using Composer, then they should have the vendor directory.

If someone is using the release zip, then they should have the vendor directory.

I think the only scenario when it might not exist, would be if they did a git clone, and forgot to run composer install. We could check if the autoload_classmap() function exists, and if not, wp_die('Please run <code>composer install</code>.') as a reminder.

Maybe also add a comment just above, explaining about the release zip, so if anyone else was "just looking at the code", they'd be suitably informed.

GaryJones commented 6 years ago

@afragen

As the plugin requires post processing it must either be installed from the release asset link or via composer.

One could immediately edit the main plugin file and decrease the version number after Installing via GitHub Updater with the immediate notice for updating. This update will pull the release asset. Any attempt to activate the plugin prior to this will result in the error above.

After install, and before it offers the user the chance to activate, could it then scan the plugin headers, and if it finds a release asset header, go ahead and run an immediate (re-)install using that instead?

afragen commented 6 years ago

@GaryJones it’s probably simpler to add a new feature to install via a URL if the user wants to use the GitHub Updater Install feature. The trick would be getting the user to use that one. It still a matter of getting the user to install in the correct manner, composer or the release asset.

https://github.com/afragen/github-updater/issues/713

bradyvercher commented 6 years ago

@GaryJones

I think the only scenario when it might not exist, would be if they did a git clone, and forgot to run composer install.

When downloading a WordPress plugin from GitHub, I think people might be conditioned to download the source zip on the releases page or from this popup, which wouldn't include the vendor directory, either:

screen shot 2018-09-05 at 9 09 54 am

We could check if the autoload_classmap() function exists, and if not, wp_die('Please run <code>composer install</code>.') as a reminder.

Checking for dependencies would work. Instead of exiting and preventing access to the site, we can probably display an admin notice with a link to the installation instructions.

bradyvercher commented 6 years ago

76 should prevent fatal errors and direct users to the installation instructions if dependencies are missing. I'm pretty sure the original issue is related, so I'll close this out. @Nobiuss, feel free to reopen this if you're still having problems.

Nobiuss commented 6 years ago

Great work guys. I will test this later.

afragen commented 6 years ago

@GaryJones @Nobiuss @bradyvercher @thefrosty GitHub Updater develop branch now has the ability to install from a zipfile, remote URL or a local file. No directions as yet, I'm interested in knowing how easy it is to figure out.

polevaultweb commented 5 years ago

If someone is using Composer, then they should have the vendor directory.

I came across this recently. I installed the plugin via Composer (whole site managed with composer.json) in the root.

And I see image

Because the plugin is looking for the vendor directory in it's directory. This wont exist as the vendor dir is at the site root.

bradyvercher commented 5 years ago

Hi @polevaultweb! SatisPress doesn't check for a vendor directory. It checks to see if the \SatisPress\autoloader_classmap() function exists. That function is defined in /src/functions.php and should be autoloaded by Composer, so if it doesn't exist, dependencies probably aren't available.

I run SatisPress on a site that's managed by Composer (SatisPress is installed by Composer) and haven't run into this issue.

If you open the /vendor/composer/autoload_files.php file, you should see an entry for .../plugins/satispress/src/functions.php. If that's missing, you might need to trying running composer install again or composer dump-autoload to see if it'll add that entry.

polevaultweb commented 5 years ago

Hmmm, i was referring to https://github.com/cedaro/satispress/blob/a9d5f353d33bef837227a7a49882abd84be08d80/satispress.php#L44 but of course that's for packaged up plugins. Weird why the composer autoloader isn't boostrapping the functions.php file - ill do some more digging. Apologies for the false alarm.