wikimedia / composer-merge-plugin

Merge one or more additional composer.json files at Composer runtime
MIT License
926 stars 160 forks source link

Autoload section from `local` doesn't replace main one #188

Open someonewithpc opened 4 years ago

someonewithpc commented 4 years ago

I'm assuming I'm doing something wrong, but the autoload section of my composer.json isn't being merged with the one in composer.local.json.

The intent is to have different plugins which are mutually exclusive and have a default. Plugin B should replace A with this .local file. In case it matters, I'm using Symfony.

# composer.json
...
    "autoload": {
        "psr-4": {
            "Plugins\\Foo\\" : "plugins/A/Foo/"
        }
    }
...
    "extra": {
        "merge-plugin": {
            "include": [
                "composer.local.json"
            ],
            "replace": true,
            "ignore-duplicates": false
        }
    }
...
# composer.local.json
{
    "autoload": {
        "psr-4": {
            "Plugins\\Foo\\" : "plugins/B/Foo/"
        }
    }
}

If I remove the duplicate from the main file, the B plugin is loaded, otherwise the A is, which isn't what I want. Am I doing something wrong?

bd808 commented 3 years ago

Autoload declarations are merged using array_merge_recursive. This merging is not affected by the extra.merge_plugin.replace feature flag. The replace flag is documented as being a control over resolving conflicting package versions.

The use case you are attempting is not currently supported by composer-merge-plugin.

someonewithpc commented 3 years ago

Is this something that could be added? Would it need a separate flag, or could replace be sort of overloaded for this effect? I'm open to making a MR, if you think this would be an appropriate feature

bd808 commented 3 years ago

I would like to see a more in-depth explanation of your use case before commenting on the suitability for adding more features to composer-merge-plugin.

As I currently understand it your use case sounds like an attempt to use composer-merge-plugin to apply DRY to the composer.json files for multiple plugins. Is that roughly correct? Can you provide more detail about the expected benefit of this for your project? Do you have any data or intuition about how many other projects could benefit from this ability to replace autoloader configuration via inheritance?

someonewithpc commented 3 years ago

I could implement it another way, but I think this would be ideal. The main idea is to allow switching which implementation corresponds to the namespace, so that different components can be used. Specifically, this would be used in GNU social to allow having DB-, REDIS-, Memcached- or STOMP- based queue system such that the end used would be able to choose which they want to use. (since GNU social is all about costumization, extensibility and the ability to run anywhere, including shared hosts).

Admittedly, there might be another approach which would be simpler, I'm open to suggestions. We already make heavy use of an event system, so this wouldn't be strictly necessary, but should help slightly with performance by removing the whole event processing chain and indirection where instead a mock implementation could be used by the core and plugins could replace it.

About the further impact, I'm not sure, I'm not aware of many projects having to have user customizable options at this sort of level, so the impact might be limited, but I think the cost of implementation and maintaining wouldn't be too big.

bd808 commented 3 years ago

The main idea is to allow switching which implementation corresponds to the namespace, so that different components can be used.

I do not understand this explanation. I understand the concept of highly customizable software products. I also understand the common methods of implementing provider interfaces in multiple competing plugins. I do not understand how using composer-merge-plugin to modify namespace registration in the PHP runtime solves some aspect of implementing or configuring such plugins.

Looking at https://github.com/someonewithpc/gnusocial I'm wondering if what you are trying to do is actually graft on configurabilty to your fork of an upstream project without that upstream being designed to be extended in this way.

someonewithpc commented 3 years ago

Sorry,i didn't mean to imply that you didn't know about those concepts.

Yes, GNU social is being rewritten based on symfony which doesn't handle extensibility in this way.

The purpose of having this "namespace overriding" would be allowing the core project to call Queue::enqueue and have that queue namespace be replaced by some implementation, as opposed to doing something like Event::handle("e queue",...). This way each plugin would be in its own folder, but define the Queue namespace.

I'm not sure how best to explain it, so it might be best for me to try to make a working example.

By the way, the mirror here in github is outdated, afaik, the canonical version is over at notabug.org

bd808 commented 3 years ago

allowing the core project to call Queue::enqueue and have that queue namespace be replaced by some implementation

This sounds like a reason to change the code in your project and not to hack things in a way that will be confusing and difficult for the end-users to manage (composer-merge-plugin is 100% a pile of ugly hacks to work around deficiencies in Composer managed projects, starting with MediaWiki).

Something Symfony's service container would be a much nicer way to implement a pluggable backend for a queue system. I am by no means a Symfony expert, but dependency injection was in part invented to support use cases of selecting an implementation of a common interface at runtime. Having a Queue service object that internally uses a "handler" implementation chosen by configuration to actually manipulate the queue would be a common pattern for a pluggable backend.