-
Notifications
You must be signed in to change notification settings - Fork 397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: always require autoload.php #519
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linting failed (1 error).
1 notice occurred in your codebase, but none on files/lines included in this PR.
We dont want to always include autoloader.php, as that should only be used in the case where you are installing s3-uploads completely standalone outside of your project using composer (with all deps installed to |
I understand, but I want to use this plugin on a site that's already got the SDK and does not manage plugins with composer. Maybe you should add a disclaimer to the readme that the plugin will throw a fatal error if you just install it from source and run composer install, and also have the SDK already loaded. It would be better to avoid the error IMO - I don't expect this type of error as a plugin installer, and the plugin is unusable in this scenario. |
@korridor hmm I'm not sure-- I had though this issue was created because |
@korridor is correct. Steps to reproduce #515 on a site that already loads the SDK via another plugin:
In this scenario, this plugin's autoload.php file is never loaded because the condition only checks that the SDK class exists, which it does but not because this plugin loaded it. But this plugin always throws a fatal error as a result, until/unless its own autoload.php file is loaded, like I've done in this PR. |
Works as a charm. I cannot see any issue with it. Maintainers, please consider this PR. |
Bump. Had this issue with the manual install build release as well. This PR fixes the issue. Please consider this PR. |
Alternatively, this fix could be applied by adding the following to // Ensure the AWS SDK can be loaded.
if ( ! class_exists( '\\Aws\\S3\\S3Client' ) ) {
// Require AWS Autoloader file.
require_once dirname( __FILE__ ) . '/vendor/autoload.php';
}
require_once __DIR__ . '/inc/namespace.php';
... |
This comment was marked as spam.
This comment was marked as spam.
@MattWilliamsDev Please do not ping people repeatedly across multiple issues, or we may need to limit your ability to comment on Human Made projects. |
@rmccue Thanks for the reply and sorry but it worked for getting eyes on this! There doesn't seem to be a way around using composer with this package. There also doesn't seem to be a working installation for people that don't already have the AWS SDK running (like this PR aims to fix). These are problems for a lot of people and they're problems that also seem to lack documentation. This solution only solves cases that still use composer, but that doesn't help for manual install because the dependencies aren't packaged with the tarball. Even when I install the AWS SDK manually and then add the steps to load the SDK without composer, this still fails because it still needs something to be autoloaded. |
I can confirm that adding @MattWilliamsDev's patch to an installation done with |
I can also confirm that @MattWilliamsDev patch works for the latest manual install. |
Thanks for the PR! We're going to close this in favour of #644. |
This fixes #515, a fatal missing class error on the site as well as in WP CLI commands with the plugin active and the AWS SDK loaded from another source.