Avoiding dependency hell

[joelwurtz]

Currently each adapter of php-http will have his own repository, i'm afraid to see a big complexity if some days an interface need to be changed and all adapter will then have to be updated and match against the new version.

My proposal is to have all the code / issue / ... on one repository and make sub view (like symfony with its components), so a PR for updating a interface will also contain the modification for all corresponding adapter / client.

[sagikazarmark]

Well, that's a point I haven't thought about so far. We could have an "adapter" package which contains all adapters, but I haven't done anything like that before.

One additional advantage I see is that this way adapters and tests can be in one repo (currently they are separated) and we can use behat. It has been a problem so far: how to use same behat tests for multiple repositories.

Anyone else? Ideas?

[hannesvdvreken]

You're talking about subsplit packages? Yes, that's a good idea, so that integration tests can reside in one repo, and all adapters can be installed separately.

In theory: good idea. In reality: could be a mess to setup. There are some tools out there to help us with that. https://github.com/dflydev/git-subsplit

[hannesvdvreken]

quick few extra questions:

• Hpw would you handle version numbers of the aggregated repository vs the separate adapter packages?
• What if we need to test both a Guzzle v5 and a Guzzle v6 adapter? Can't install both versions at the same time.

[sagikazarmark]

In theory: good idea. In reality: could be a mess to setup.

My concerns exactly.

Hpw would you handle version numbers of the aggregated repository vs the separate adapter packages?

I guess the separate adapters can have their own version number. Or versions could only be meaningful in the main repo, separated adapters would just follow it. Eg. changes in a few adapters. Release on the main package with a central changelog. Split and tag the same releases.

What if we need to test both a Guzzle v5 and a Guzzle v6 adapter? Can't install both versions at the same time.

That's a hard question. The only solutions I can think of:

• in cases like this the main adapter package holds only the latest version (like guzzle6), and the rest goes to a separate package (legacy?).
• another solution is that during testing, we add this into the build matrix, so that we can test against multiple versions (I think this is how it works in Ivory right now, but I am not sure)

[hannesvdvreken]

and the rest goes to a separate package (legacy?).

Ok for packages that officially deprecate their old major versions.

we add this into the build matrix

Imagine the build matrix with 4 PHP versions (5.5, 5.6, 7.0, HHVM), times 2 Guzzle versions, times 2 for lowest and highest. That's 16 entries in the matrix, very quickly.

[sagikazarmark]

Ok for packages that officially deprecate their old major versions.

But as far as I know, Guzzle 5 is still supported.

Imagine the build matrix with 4 PHP versions (5.5, 5.6, 7.0, HHVM), times 2 Guzzle versions, times 2 for lowest and highest. That's 16 entries in the matrix, very quickly.

I know, but I don't have any better ideas. Optionally we could run Guzzle5 tests only on 5.5, so that it only adds one extra build to the matrix.

[joelwurtz]

We could have the following workflow :

The master repository (i.e. php-http/php-http) is the reference for issue and source modification, namespace in the src will help to split into different repository :

• Http\Client -> php-http / client
• Http\Socket -> php-http / socket-client
• Http\Guzzle5 -> php-http / guzzle5-client
• Http\Guzzle6 -> php-http / guzzle6-client
• ....

Each of this namespace / repository have its own composer / test file (there is no composer.json at the root of the master repository).

The php-http master repository may have a tests directory for integration (but i dont think this will be necessary)

It will also have the .travis.yml where the test script will do the following thing :

• For each split
• cd into the directory
• composer install
• run tests

Then we can add a post test script which will update all the subtree split if the run is against the master branch

For the build matrix, we can also have the test script only running test for a specific repository given an env variable like :

matrix:
    include:
        - php: 5.6
          env: 
             - GUZZLE_VERSION=6.0
             - PHP_HTTP_TEST_REPOSITORY=php-http/guzzle6-client
        - php: 5.6
          env: 
             - GUZZLE_VERSION=6.1
             - PHP_HTTP_TEST_REPOSITORY=php-http/guzzle6-client

[joelwurtz]

Each repository will also have an override of the autoload provided by composer to use classes of php-http from the relative directory instead of the vendor one

[dbu]

i am actually -1 on this. my reason is that we need to strictly semver the interfaces anyways. the idea (at least to me) is that people can also write their own adapters or have their clients implement the interfaces directly. at that point we can't change interfaces without major version changes anymore.

having separate repositories that properly track which version of the client they support is not more effort than doing it all in the same. and the versioning of the adapters becomes a lot less confusing that way.

we could have a separate repository with the generic part of the adapter testing infrastructure. or put that test part into the utility repo if we separate it from the api repo.

[sagikazarmark]

Well, I see the point in the one repo and the separated repo solution as well. The question is that merging everything into one repo or having separate repos is easier to maintain and use. Also, the one repo solution comes with a few downsides (messed buildmatrix, directory structure, composer configurations and the subtree split itself).

[joelwurtz]

With more thinking, i think also there is more downsides than upside.

The dependancy tree should not be very complex, as each adapter / client will only depend on the master client (interface) there should be no interaction between them (or very limited)

[hannesvdvreken]

Dependency graph should be like this:

(from this blog post: http://blog.madewithlove.be/post/http-client-for-sdks/)

[hannesvdvreken]

All arrows go up, as they only depend on more stable packages, or higher level abstractions.

[joelwurtz]

And, if i'm correct, with the current terminology branch adapter-client no longer exists (adapter is replace by client)

[sagikazarmark]

Adapter client originally implemeted HttpMethodsClient and accepted an Adapter. It could still be implemented. However I would rather replace it in the dependency tree with methods-client-implementation.

[beberlei]

I agree with @joelwurtz - why not make this a single repository? This code here is just simple adapters, blowing this up to so big proportions only hinders maintenance and prevents user adoption because of complexity.

See doctrine/cache which is essentially also a repository of adapters.

[dbu]

@beberlei so would you go with a subtree split instead? it is important that users can get just the interfaces and the adapter they want. also, for proper dependency modeling, the main repo can not depend on all guzzle implementations and every other http client, but the adapters should specify their client implementation as dependency, not just use "soft dependencies" with composer suggestions.

[beberlei]

The argument "API changes" is a very bad one, if you even consider API changes then why should I even use this? The stance here should be absolute and perfect API stability.

About testing, this can be easily done on Travis with the new composer related flags. It requires a little work, but that is perfectly acceptable.

About dependencies: The composer.json should not have any matching dependencies in require. If you start doing this in multi package setup, you get a messy version check problem, if third party libraries come into the mix, instead it should be something like this in a monorepo, allowing all packages for which an adapter exists:

{"require-dev": {"guzzle/guzzle": ">=3,<=6"}

@dbu Why subtree split? This repository will not hold much code, i would just ship it a s a single package. Why is it important to get only the adapter they want? You are adding complexity for something that is ~200-300 LOC (maybe more).

Proper dependency modelling will cause problems when third party packages use http clients explitliy again. It will not solve the the issue, because people will start adding dependencies to the implemtations and break your idea completly in a very easy way:

{"name": "acme/third-party-api", "require-dev": {"php-http/guzzle-v6"}}

If just two packages do this with incompatible versions, then you have the same problem again.

You can only fix this problem, by not relying on dependencies at all and delegate this to the user.

[dbu]

i am not 100% sure what is better, but talking with people like @matthiasnoback got me the idea that implicit dependencies are bad and it would be better if people would require php-http/guzzle-v6 which in turn requires guzzle.

3rd libraries should never depend on a concrete adapter, but only on php-http/client-implementation. this means using the library, you are force to chose a client or adapter that provides this virtual package. that way, there are also no conflicts on versions. only actual projects should choose the adapter implementation, every reusable code must only specify the virtual package.

that said, if we really need that little code and can make it work, having just php-http/httplug as requirement of the reusable libraries and the project needs to pick that and a client implementation supported by the adapters, it might work just as well. either way there is a wtf moment when you don't follow the doc: either composer can't find the requested virtual package, or you can't bootstrap because there is no adapter / the factory method finds nothing that is supported.

@matthiasnoback would love to hear what you think about this dilemma?

[beberlei]

@dbu Implicit dependencies are bad, but explicit dependencies are what causes the problem here that you want to solve. You cannot solve it by introducing even more abstraction layers of explicit dependencies.

Edit: "should not" is not going to cut it, people will do this wrong.

[dbu]

we try to document it here: http://php-http.readthedocs.org/en/latest/#installation-in-a-reusable-package

it does not look that complicated to me. i am not sure that the argument that people get things wrong counts against this. there is always somebody doing stupid things, but i think with this model, at least those who got the concept can do it properly.

the guzzle problem is solved by only depending on adapter-implementation instead of guzzle6.

[hannesvdvreken]

We're actually moving away from a repository with all adapters in it. See the reasoning behind it in this issue: https://github.com/egeloen/ivory-http-adapter/issues/104

Closing this issue for now, but can be reopened anytime.

[matthiasnoback]

@hannesvdvreken Nice :)

Because here and in the other thread Composer's virtual packages were mentioned, I'd like to add a note that in general I'm not a big fan of it. When installing an abstract core package with just an interface, you should not force users to also install an adapter. Maybe they write their own adapter and then you basically force them to modify their project's composer.json and add "provides": "adapter-package" to it. See also a previous post about this subject: http://php-and-symfony.matthiasnoback.nl/2014/10/composer-provide-and-dependency-inversion/

The power in abstract, stable packages is that they don't try to tell you anything about the implementations required to actually use the package.

[dbu]

thanks for this input @matthiasnoback - i guess we should think hard if we want virtual packages or just implementations. looking at the https://github.com/dbu/HttpClientBundle/pull/6 for example, it would be weird to not require an implementation. the error will only pop up when trying to run things, not with composer require php-http/client-bundle. not sure how bad that is.

@matthiasnoback, do you also have an opinion on the discussion whether the adapters should each be in a separate repository, or bundle all we can together with the contract? or is it a good thing if the adapter depends on the actual implementation (e.g. guzzle 6 adapter depends on the guzzle/guzzle6 package?)

[matthiasnoback]

@dbu If the bundle forces you to pick one implementation by means of a configuration key for example, you can require the user to provide an implementation (since you know about all the possible implementation packages). Like I mentioned in the post I linked to, this makes sense to me (since you maintain all possible implementation packages too). If however there is a chance that people will want to provide their own implementations, I wouldn't force them to "provide" the virtual package as well. But then your bundle configuration needs to support that too of course.

I totally agree with you that each adapter should be in a separate package for all the reasons mentioned in my book ;) The main reasons in short:

• Dependencies are dependencies, if you can't run all the code in a package without some other packages, those packages are the true dependencies.
• If you only use the "suggest" key, you can't tell people which version constraints they should apply when adding the adapter package to their project.
• Having separate adapter packages allows you to work with branches for special cases. You won't need to hack your code to support incompatible versions of the dependencies (like for example here: https://github.com/Seldaek/monolog/blob/master/src/Monolog/Handler/MongoDBHandler.php#L34).

[sagikazarmark]

Wow, quite long discussion :smile:

since you know about all the possible implementation packages

That's not true. We provide some major ones. However in some cases you might want to write your own (for whatever reasons). You can even do it in your application: you simply have to provide the virtual package in it. (Yeah, I know you mentioned it, but I think it is simply not our responsibility to decide whether it is a recommended/non-recommended thing or not.). Virtual packages are like interfaces for me in dependency handling.

I don't think requireing the virtual package provide is a big deal. Why is it a problem? It's like you say: "I implement the methods of this interface, but I don't actually want to implement the interface"

[matthiasnoback]

@sagikazarmark The point being: if a package provides just an interface it shouldn't care about anyone actually providing the implementation, or you loose the whole "dependency inversion" aspect of it. In other words: the package containing the interface doesn't depend on an implementation, it's exactly the other way around.

[hannesvdvreken]

You're right. Does the interface package really "require" an implementation?

[xabbuh]

I think it's not the interface package that requires an implementation (it would be quite useless to just have it hanging around, but that doesn't count). However, it's the consumer that requires an implementation. Imagine I wrote a library that required to perform some HTTP requests, I would depend on the interface package as well as on the virtual implementation package (my library would not be usable without an implementation).

[matthiasnoback]

@xabbuh Right, right, maybe I misinterpreted the discussion then. I was saying that:

• The interface package should not require a virtual implementation package
• Adapter packages should be separate from the interface package, with actual, not suggested, dependencies
• If you maintain something like a bundle which needs both the interface and the implementation package, then you could require a virtual implementation as well.

Strictly speaking the bundle doesn't need an implementation package, just an implementation (class). But anyway, if this is the only way in which virtual package requirements are used, then I'm happy with that :)

[sagikazarmark]

The interface package should not require a virtual implementation package

It does not. Your package, which requires the interface should also require an implementation.

Adapter packages should be separate from the interface package, with actual, not suggested, dependencies

Adapter packages require the underlying HTTP Client implementation and the interfaces. It also provides the virtual package.

If you maintain something like a bundle which needs both the interface and the implementation package, then you could require a virtual implementation as well.

This is what happening. :wink: But to make it work, adapters SHOULD provide the virtual package first.

[matthiasnoback]

@sagikazarmark That's great, hopefully I've not been confusing anyone. Great work.

[sagikazarmark]

Actually I was confused a bit, because IIRC I learnt virtual packages from your article. :smile: And it seemed you are against using virtual packages at all.

[matthiasnoback]

@sagikazarmark Sorry about that then!