Add a mechanism to exclude packages from processing

[Konamiman]

Here we add an optional excluded_packages configuration key, which is an array of package slugs. Any package included here won't be processed by Mozart: it won't have its namepsaced changed and won't be moved from its original location. Namespace references to these packages won't be changed either.

This is useful if there are dependent packages whose namespace shouldn't be changed, for examle psr/container. Example composer.json:

{
    ...
    "extra": {
        "mozart": {
        ...
            "packages": [
                "league/container"
            ],
            "excluded_packages":[
                "psr/container"
            ]
        }
    }
}

Note that this pull request is similar in intent to the one to skip namespaces by leoloso, but in this case we are excluding whole packages, not namespaces.

[coenjacobs]

Hi @Konamiman, thank you for this contribution to Mozart! I'd like to understand use cases for new features, such as this one, before merging them in. Not because I'm opposed to having new features, but Mozart is made with a specific goal in mind and I don't want it to deviate it from that goal too far. Can you elaborate on why you'd want to ignore a specific package from being replaced and moved?

[Konamiman]

Hello! Sure thing.

I work at Automattic and my motivation for this feature is being able to use PHP League's container in WooCommerce. We introduced it a few months ago but we had to revert it since it conflicted with a plugin using a different version of the same package. In order to reintroduce it we needed to change its namespace at build time, for now we're using a hacky script for that but Mozart provides a cleaner way.

League's Container, however, depends on the psr/container package, since it implements PSR-11's ContainerInterface. This is a static package that is just defining a standard, and it's safe to leave it unchanged since it's not going to change.

There is another reason for leaving this package unchanged, though. League's Container has a feature named delegate containers, by which a secondary container can be registered in case the main one can't resolve a given class.

We want to allow WooCommerce plugins to register their own PSR-11 containers (be those League's or others with similar functionality), and be able to use ours as a delegate container. But this will only be possible if our container implements Psr\Container\ContainerInterface, and not WooCommerce\Vendor\Psr\Container\ContainerInterface. Thus the need to exclude the psr/container package from the namespace changing process.

And that's my story. For now I'm using my own fork with the functionality of this pull request merged in, but of course I would prefer to use the official Mozart package. 🙂

[BrianHenryIE]

I'm a bit late to the game, but just a note to say that was already possible using override-autoload. I think this is functionally the same:

{
    ...
    "extra": {
        "mozart": {
        ...
            "packages": [
                "league/container"
            ],
            "override_autoload": {
                "psr/container": {}
            }
        }
    }
}

[coenjacobs]

@BrianHenryIE I agree, this is functionally the same, but... They are two separate things, that can lead to the same end result. While this new exclude_packages parameter is here to explicitely deny that package from ever being touched by Mozart, the override_autoload parameter can be used for a multitude of purposes. Not only is this easier to explain, it's also easier to document its intended behaviour. In my personal opinion, these are fine to coexist next to each other.

[BrianHenryIE]

Agree

[Spreeuw]

Am I missing something or is this solution not recursive? It seems that tertiary dependencies still get moved even when excluded.

Edit to provide some context:
A package I was using uses Guzzle, which in turn uses psr/http-client, and this resulted in errors such as:

Fatal error: Could not check compatibility between MyNamespace\GuzzleHttp\Client::sendRequest(Psr\Http\Message\RequestInterface $request): Psr\Http\Message\ResponseInterface and MyNamespace\Psr\Http\Client\ClientInterface::sendRequest(MyNamespace\Psr\Http\Message\RequestInterface $request): MyNamespace\Psr\Http\Message\ResponseInterface, because class Psr\Http\Message\ResponseInterface is not available

I then tried adding psr/http-client and psr/http-messages to excluded_packages but that didn't change anything. adding it to override_autoload worked fine (as did applying #82), resolving this particular issue... although Guzzle still wasn't functional.

[coenjacobs]

@Spreeuw Good find. Can you confirm that this still is happening on the current master branch, now that I have merged in the pull request you've referenced and the other related parts to that as well? I have a hunch that I know where this might be a problem, but since you've got the test bed setup, it might be easier for you to verify? If it still results in the same problems (either Guzzle not working, or the packages not being ignored), then please post this in a separate issue. This one is closed/merged and the discussion gets lost easily. Thanks for reporting!

[Spreeuw]

Thanks. I understand this may not be the best place for discussion, I just wanted to mention the thing about not being recursive here and then ended up with that much larger context edit :)

With current master (5d8041f), the excluded_packages stil doesn't work for me on the tertiary dependencies, i.e. psr/http-client and psr/http-messages still didn't get excluded.

That fatal error above does not occur as a result of #82, so that's a good thing (and removes the requirement for me to exclude them). Guzzle still doesn't work though, throwing: Uncaught Error: Call to undefined function MyNamespace\GuzzleHttp\Psr7\stream_for()

TBH I'm not sure what exactly the issue is here, and I didn't want to post an issue just saying "Guzzle is not working". I may open an issue if I have more information to give you.

[coenjacobs]

@Spreeuw I have made a ticket containing the details and questions I need answering to attempt to fiix this. I'm fairly certain that I know what to look for, but am also uncertain about your entire configuration. If you could provide some more details in #88, that would be very much appreciated. 👍

[XedinUnknown]

If I understood correctly, this will simply not process files in the excluded packages' folder. I haven't read the example with WooCommerce yet, but I am finding it hard to imagine cases where this would be desirable. Exclusion is useful based on FQNs, and if an FQN gets prefixed, then it should get prefixed in all packages. If it is excluded, then it should not get prefixed in any packages. IMHO.