search

php-http / discovery

Finds installed clients and message factories
http://php-http.org
MIT License
1.26k stars 47 forks source link

Auto require package I do not need #236

Closed bobzhangyong closed 1 year ago

bobzhangyong commented 1 year ago

PHP version: x.y.z (hint: php --version)

PHP 7.4.11 (cli) (built: Mar 4 2022 15:04:46) ( NTS ) Copyright (c) The PHP Group Zend Engine v3.4.0, Copyright (c) Zend Technologies

Description

My project need a private package petrel/opentelemetry, this package require open-telemetry/opentelemetry version 0.0.17

open-telemetry/opentelemetry require php-http/discovery, php-http/async-client-implementation, psr/http-factory-implementation

In my project ready have private package petrel/http-client provide php-http/async-client-implementation and psr/http-factory-implementation

But when I run composer update, this packages will add to my project automatic, and change my composer.json 

        "guzzlehttp/promises": "^1.5",
        "nyholm/psr7": "^1.7",
        "php-http/message-factory": "^1.1",
        "symfony/http-client": "^5.4"

I think the problem is php-http/discovery do not check private provide packge.

Any idea to fix this problem? I can only change petrel/opentelemetry, can not change open-telemetry/opentelemetry

Thank you

nicolas-grekas commented 1 year ago

The role of the plugin is to provide some nice DX so that e.g. this line works out of the box: https://github.com/open-telemetry/opentelemetry-php/blob/5f318cdb7ac299054c23996b3276801849551538/src/SDK/Common/Adapter/HttpDiscovery/PsrClientResolver.php#L27

If you don't want that behavior, you should disable the plugin by running:

composer config allow-plugins.php-http/discovery false
bobzhangyong commented 1 year ago

We have more than 100 projects use petrel/opentelemetry package, it's not easy to inform all user add config in their project composer.json conf Do you have any other idea?

nicolas-grekas commented 1 year ago

I don't think there is a solution that belongs to the petrel/opentelemetry package: it's very possible, in theory at least, that open-telemetry/opentelemetry would be required by another dep that doesn't rely on the wiring provided by petrel/opentelemetry, so that the discovery mechanism would still need a package it knows how to use to work properly.

This means that to me, only the root package can know how to solve this. One way is to disable the plugin. You could very well document that in the readme of your private package. If people don't read it, it's not a big deal either. They'd just get a maybe-unused dep.

The alternative, but I would reserve it to the root package also, would be to build on https://github.com/php-http/discovery/pull/232 so that when all interfaces required to provide a virtual *-implementation are listed in the extra.discovery entry, then the plugin shouldn't need to install anything. (But this wouldn't solve your concern.)

bobzhangyong commented 1 year ago

Each package that implements php-http/async-client-implementation will set the provide field in their composer.json file. When php-http/discovery looks for dependencies, it can check all the provide fields and if there is an implementation for php-http/async-client-implementation, there is no need to add a new package.

Can we do like this ?

nicolas-grekas commented 1 year ago

I thought about that, but there are two issues:

bobzhangyong commented 1 year ago

Your concerns are reasonable. Well, I don’t have a better solution to this problem either. I will close this issue. Thank you for your reply.

nicolas-grekas commented 1 year ago

Thanks. I created #237 to keep track of the proposal in https://github.com/php-http/discovery/issues/236#issuecomment-1526159908

nicolas-grekas commented 1 year ago

See https://github.com/php-http/discovery/pull/239 ;)