The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props leonidasmilossis, davidbaumwald, afragen.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Looking at the current patch, I'm wondering if there should be any sort of guardrails on the $source that's returned from the new filter to ensure it's either a WP_Error or a (string) $path and that path is in the WP_CONTENT_DIR, WP_PLUGINS_DIR, or whatever path we'd want this constrained to. @afragen and I discussed this a bit ad WCUS CD.

Extra props @afragen.

Personally, I'd rather not guard against $source being a part of any specific path, which would limit the third-party author's flexibility. The filter is added to grant them that flexibility, is there a reason I miss to restrain that?

In any case, I've now added the guardrails of validating against being either a string or an error, which is already an improvement.

As for guarding against the path being in WP_CONTENT_DIR, we didn't do that before and the path was already filterable, so I'm not sure if we need to add this now. I'm interested in what you think @afragen.

I also pushed a change unifying the two new filters into one, as per feedback from @afercia .

I need to review further but initially I don't think the final return should be WP_Error. It should be $source.

Is it even possible for $source to be a WP_Error going into this filter? I'm not sure that check is needed.

Is it even possible for $source to be a WP_Error going into this filter?

Indeed, I dont think it's possible to be an WP_Error going into this filter. It can be a WP_Error going out of it (which is basically the centerpiece of how we try to give plugin/theme authors a way for their extra custom checks).

I'm not sure that check is needed.

I'm totally fine with removing the check, I was mostly trying to follow the advice above and guardrail the $source.

Which is basically why I'm returning a WP_Error if coming out of the filter we get a non-string, non-WP_Error value, since that would indicate wrong usage of the filter. But maybe I missed the point of the advice?

In any case, thanks for responding :)

I think the only way a WP_Error returns from the filter is is someone uses the filter to return it.

If we move up the filter to just before the checks for is_wp_error() in the functions I think this can be greatly simplified.

Will need to account for variables passed.

Honestly, I'm still trying to see why upgrader_pre_install won't work for you. I say this as someone who has extensively worked in code related to the upgrade/install component.

The use cases outlined in https://core.trac.wordpress.org/ticket/62043 could work very easily with that existing hook.

I'm still trying to see why upgrader_pre_install won't work for you

Because in order to achieve what this new filter allows us to do, we need access to the get_plugin_data() values and, from what I can tell, those are not readily available in upgrader_pre_install.

To give an example of how that new filter could be used:

\add_filter( 'upgrader_checked_package', [ $this, 'check_required_version' ], 10, 3 );

public function check_required_version( $source, $info, $package_type ) {
	$requires_main_plugin_version = ! empty( $info['Requires Main Plugin'] ) ? $info['Requires Main Plugin'] : false;

	if ( ! $this->check_requirement( $requires_main_plugin_version  ) ) {
		$error = \sprintf(
			\__( 'The Main Plugin version on your site is %1$s, however the uploaded plugin requires %2$s.', 'text-domain' ),
			\MAIN_PLUGIN_VERSION,
			\esc_html( $requires_main_plugin_version )
		);

		return new WP_Error(
			'incompatible_main_plugin_version',
			\__( 'The package could not be installed because it\'s not supported by the currently installed Main Plugin version.', 'text-domain' ),
			$error
		);
	}

	return $source;
}

private function check_requirement( $required_version ) {
	if ( $required_version === false ) {
		return true;
	}

	return \version_compare( \MAIN_PLUGIN_VERSION, $required_version . '-RC0', '>=' );
}

(consider that the Requires Main Plugin info has been added via the extra_plugin_headers filter)

So, the above code in the main plugin would prevent installations/upgrades of its add-on, if that add-on had a required version for the main plugin.

I think the only way a WP_Error returns from the filter is is someone uses the filter to return it.

Yes, that exactly would be the way for that plugin to prevent an installation of an incompatible add-on version.

If we move up the filter to just before the checks for is_wp_error() in the functions I think this can be greatly simplified.

I'm failing to see how that could be the case 🤔

How does the following not achieve your goal?

Using get_file_data() just grabs the header data from the plugin. Requesting specific data in $header_arr only returns that data.

add_filter(
	'upgrader_pre_install',
	function ( $bool, $hook_args ) {
		$header_arr = array( 'RequiresMainPlugin' => 'Requires Main Plugin' );
		$plugin_headers               = get_file_data( trailingslashit( WP_PLUGIN_DIR ) . $hook_args['plugin'], $header_arr );
		$requires_main_plugin_version = ! empty( $plugin_headers['RequiresMainPlugin'] ) ? $plugin_headers['RequiresMainPlugin'] : false;
		// Add check_requirement logic. Return WP_Error if fails.

		return $bool;
	},
	10,
	2
);