Core: Move some dev-deps on suggest

[Taluu]

On the last clone I made just to check for things, I couldn't run composer install because I don't have mongo (or its ext) installed.

If you want to contribute to API Platform, you shouldn't force the dev to install these dependencies (or worse, go through --ignore-platform-reqs which is something that should never be used, and is sure to make the project go :boom: ). These should be "suggested" instead. So that if the contributor want to contribute to the odm / orm bridge (for example), these should be installed on his part.

The dev dependency should really be limited to just things such as phpstan, phpunit and dev tools used.

This kinda related to #3122. If you don't limit, don't ask the contributor to use something else.

[teohhanhui]

The mongodb extension is required to make phpstan work. Anyway, it's very easy to install the extension, so not a real concern.

We've tried the alternative. It's worse, because there's no way to make phpstan work locally, other than to modify composer.json. (Really bad!)

[Taluu]

No, I won't install mongodb as I'm not using it and I don't want to maintain upgrading it whenever I need to work on apip. The current "solution" is much worse.

It's really like you would be requiring (not on dev level) these packages just because you have bridges, as you would with the future laravel one : it is a huge source of conflict and will just push contributors away.

[teohhanhui]

We maintain an integration with mongodb, so it's only natural that you need to install it if you want to work on API Platform. As I've already explained, phpstan needs it, and we don't want that to be broken for local development.

[Taluu]

There's an integration only through a bridge. If I don't use mongodb and nothing I want to contribute to hits mongodb, it shouldn't be an obligation to have it (as you said, it's an integration, it's not a hard requirement ; it's already in the suggestion section anyway).

E.g currently, if you install with --ignore-platform-reqs, you are installing the newest proxy manager bridge which requires 7.4 and everything goes :boom:. So yeah, not the smartest move if you ask me.

You'll lose way more contributors that way.

[dFayet]

There's an integration only through a bridge.

I think the same discussion could be applied on doctrine, elasticsearch, ....

If I only want to contribute to the core, I shouldn't need to install the whole bridges family as require.

[teohhanhui]

That's clearly not feasible / maintainable. We chose to go with a monorepo, so that's just how things must remain. However, we would not require installing some obscure extension. The mongodb extension is very easy to install. It's your choice, if you refuse to install it, nothing stops you from still contributing and just running the tests with our CI setup.

[Pierstoval]

Isn't there a way to skip tests or scripts that require mongo if mongo is disabled?

If phpstan requires the extension, maybe a small guide on how to execute it without mongo installed by creating a phpstan.neon file with the correct config?

[Taluu]

@Pierstoval The tests are already skipped if the dependencies aren't met IIUC.

[teohhanhui]

If phpstan requires the extension, maybe a small guide on how to execute it without mongo installed by creating a phpstan.neon file with the correct config?

Having such combinatorial explosion is very bad for maintainability (i.e. phpstan.neon without A, phpstan.neon without A + B, ...). I'm strongly against this.

[teohhanhui]

We skip mongodb tests on PHP 7.1 because it's not supported by DoctrineMongoDBBundle.

[Pierstoval]

At least composer install could still work if composer.json is updated with the config.platform.ext-mongodb="1.5" option

[Taluu]

@teohhanhui that's why it should be on suggested actually. And in the tests, whenever there is a mongo dependency, manually install the mongodb, the odm dependency, ... and so on (through pear install, composer require, ... and so on)

On the phpstan job (it should be ran only on 7.2 and one build, it should be the same for any other ones), add the mongodb dependency.

[teohhanhui]

The benefit of having phpstan working locally outweighs the minor inconvenience of having to install the mongodb extension (which is really easy to do, by the way).

[Taluu]

This ain't just mongodb. What will you do when e.g we want to have support for an obscure extension for some reason (e.g let's say amqp) ? will you force the contributors to install it ?

[teohhanhui]

Any hypothetical integration with amqp wouldn't have the same scope as the mongodb integration, so I think the point is moot.

[Taluu]

You didn't get the point. The problem still remains : why the hell would I want to have to install such things (even the pdo if you will) when I do not even care about the doctrine bridges ? This shouldn't prevent me from contributing to the core.

As already said, these are suggestions. so they should stay as theyr are : suggestions. If you want to contribute back to the different bridges, then you are free to install the stuff you need to make it work.

Having phpstan work locally doesn't outweight at all the installation of the extension. Because if you install the extension, then you need mongodb. If you do not have for some reason php 7.2, you can't even install the project as doctrine odm (and other doctrine dependencies) need at least 7.2 (not mentionning the proxy manager which needs 7.4 on the latest versions).

As I already said, yes you can add the platform hack. Yes, you can use the --ignore-platform-reqs, but then that fucks up the entire thing, e.g running on < 7.3 a lib that needs 7.4 (the proxy manager), or running a 7.1 for some reasons on doctrine latest which needs at least 7.2 and the object typehint it will make everything go :boom:.

And so on : your choice.

[teohhanhui]

The point has already been made: what you've suggested is not maintainable.

[dunglas]

We tried hard to find another solution than requiring this extension, but they are all worst and create troubles. While this extension isn't required to use API Platform, it is to contribute because of PHPStan, and because the whole point of having a monorepo is to be sure that all tests for all integrations still work when doing changes to core. mongodb is a development dependency of this monorepo, and must be declared as so. Doing otherwise makes the more CI complex (and harder to maintain) and prevents PHPStan to work properly. I would like to find a better solution, but I've none in mind.

I recognize that this dependency makes it harder for new contributors to work on API Platform, and so I'm open to any solution to improve this.
Maybe should we reconsider @alanpoulain's suggestion to provide a dev Dockerfile for contributors to be able to run all the tests locally in a single command?

[dunglas]

(Unlocking because the discussion is interesting and we should find a solution to the main issue, but moving these deps to suggest doesn't look OK as is to me)

[Taluu]

A docker image would be fine with all dependencies IMO. As long as the basic git clone doesn't have to force install all the related bridge dependencies...