Skip to content

Autoloader: Detect filtering of active_plugins#17174

Merged
jeherve merged 1 commit into
masterfrom
add/autoloader-plugin-cache
Sep 17, 2020
Merged

Autoloader: Detect filtering of active_plugins#17174
jeherve merged 1 commit into
masterfrom
add/autoloader-plugin-cache

Conversation

@ObliviousHarmony
Copy link
Copy Markdown
Contributor

Changes proposed in this Pull Request:

In woocommerce/woocommerce#27608 it became apparent that plugins can filter active_plugins in a way that prevents some plugins from being detected by the autoloader. This PR addressed the issue by caching the list in should_autoloader_reset() as an additional means to evaluating whether or not manifests should be processed again.

Jetpack product discussion

N/A

Does this pull request change what data or activity we track or use?

No

Testing instructions:

  • Add this autoloader-testing.php file to mu-plugin. It sets up autoloader globals in a way that pretends the autoloader has been successfully loaded. Without this PR this will result in the entire site becoming inaccessible due to class loading errors, but with this PR, it will work as-expected.
<?php
/**
 * Plugin Name: Autoloader Testing
 * Description: This plugin helps.
 * Version: 1.0
 */

 defined( 'ABSPATH' ) || exit;

// Set a version for the autoloader so that Jetpack won't attempt to
// reset the autoloader.
global $jetpack_autoloader_latest_version;
$jetpack_autoloader_latest_version = '2.2.0.0';
global $jetpack_autoloader_cached_plugin_paths;
$jetpack_autoloader_cached_plugin_paths = array();

Proposed changelog entry for your changes:

  • Autoloader: Added handling for filtered active_plugins options that would otherwise have left classes out.

@github-actions github-actions Bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 15, 2020
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Sep 15, 2020

Scheduled Jetpack release: October 6, 2020.
Scheduled code freeze: September 29, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17174

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 948fa32

@jeherve jeherve added [Package] Autoloader [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended labels Sep 16, 2020
kbrown9
kbrown9 previously approved these changes Sep 17, 2020
Copy link
Copy Markdown
Member

@kbrown9 kbrown9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me! Thanks for the test plugin.

It looks like there's a problem with the E2E tests which has nothing to do with this PR, and we're working on a PR to fix them. We'll rebase/retest/merge this PR as soon as that's fixed.

@kbrown9 kbrown9 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Sep 17, 2020
@jeherve jeherve added this to the 9.0 milestone Sep 17, 2020
@jeherve jeherve merged commit 6903d4f into master Sep 17, 2020
@jeherve jeherve deleted the add/autoloader-plugin-cache branch September 17, 2020 17:59
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 17, 2020
jeherve added a commit that referenced this pull request Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [Package] Autoloader [Status] Needs Package Release This PR made changes to a package. Let's update that package now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants