aspirepress / aspireupdate

A plugin that allows for rewriting the URLs used to fetch updates from WordPress.org to some other endpoint
GNU General Public License v2.0
29 stars 20 forks source link

Add support for must use state of AspireUpdate #79

Open asirota opened 2 weeks ago

asirota commented 2 weeks ago

Introduce a flag to enable must use loader file. @costdev and @afragen made a generic mu-loader plugin that loads plugins from wp-content/plugins as mu-plugins yet still allows them to be updated.

https://gist.github.com/afragen/9117fd930d9be16be8a5f450b809dfa8

Flag is a Boolean constant AP_MUST_USE to enable this functionally.

asirota commented 2 weeks ago

I noticed the license of this code is MIT. Do we want to mix licenses in one project?

namithj commented 2 weeks ago

We won't use that code

asirota commented 2 weeks ago

@afragen why is the code MIT licensed? Why license at all and not just submit to the project as gpl?

afragen commented 2 weeks ago

MIT license is the least restrictive license.

afragen commented 2 weeks ago

https://opensource.org/license/mit

costdev commented 2 weeks ago

FYI: This code was written months ago, and licensing it with the least restrictive license made sense to ensure that it could be used without concerns.

@namithj Can you expand on "We won't use that code"?

namithj commented 2 weeks ago

We need to build a class and our scenario just focuses on this plugin to follow the coding convention used elsewhere in the plugin.

Our use case is way simpler

afragen commented 2 weeks ago

Just some background on the mu-loader.

https://core.trac.wordpress.org/ticket/60504

https://core.trac.wordpress.org/ticket/60692

costdev commented 1 week ago

I'm having second thoughts on how this should be implemented.

  1. A Must-Use plugin is a serious decision by a site owner.
    • They should only be making a plugin Must-Use if they have the basic filesystem knowledge to be able to rectify it.
  2. Adding checks in AspireUpdate to copy a Must-Use loader into the wp-content/mu-plugins directory, or remove it, depending on the status of the AP_MUST_USE constant isn't clean:
    • An extra page refresh may be required when viewing the Installed Plugins list. A site owner may not realise to do this and may think they've done something wrong or that the feature isn't working.
    • Write permissions won't necessarily be available for the mu-plugins directory.
    • AspireUpdate can't cover all bases, such as if the plugin is deleted via FTP/SSH or by a future bug in WordPress Core.

As an alternative, I think we should include the MU Loader file in AspireUpdate/mu/aspire-update-mu-loader.php, with the necessary changes to load AspireUpdate as a Must-Use plugin, and provide instructions in the documentation. It's much safer to do it this way.

namithj commented 1 week ago

This was my original position on this matter, MU should only be for people who know what they are getting into and know how to get out of it if things go wrong. Let's just provide the loader and instructions on how to set this up as an MU plugin.

asirota commented 1 week ago

Sounds good. I assume this is the loader code @afragen provided.

https://gist.github.com/afragen/9117fd930d9be16be8a5f450b809dfa8

I believe I can add the path to the plugin loader file into the array as per comments.

Other than placing it in the appropriate directory, how should it be called from the plugin?

afragen commented 1 week ago

Yes it's the same loader code, though technically @costdev and I both wrote it. 😉

costdev commented 1 week ago

@asirota Other than placing it in the appropriate directory, how should it be called from the plugin?

No need for it to be called from the plugin. Once the site owner places the file in the wp-content/mu-plugins directory, Core will load the Must-Use loader file on the next page load.

costdev commented 1 week ago

@asirota I've added a PR as there were some more minor changes needed (such as changing the plugin name and description headers, the namespace to avoid clashes, the PHPCS exclusion, etc).