composer / composer

Dependency Manager for PHP
https://getcomposer.org/
MIT License
28.49k stars 4.52k forks source link

Add require-tools section and handle scoped vendor dirs #9636

Open nicolas-grekas opened 3 years ago

nicolas-grekas commented 3 years ago

This issue is a follow up of https://github.com/composer/composer/issues/5390#issuecomment-463625428

Composer has become the primary installation method of libraries and is so good at it that it is pretty commonplace nowadays to also install tools like static analyzers or testing frameworks with it.

But this comes at a price: tools are micro-applications with a lot of dependencies, and those dependencies can often be found in your application, or in other tools.

Every time you add one of them to your toolchain, your application becomes more constrained, and very often, for no good reason, because in most cases, you do not reference classes or interfaces from those tools in your code.

Soon enough, you find yourself in the dependency hell Composer is supposed to protect you from.

I'm dreaming of a new section in our composer.json files, called require-tools, that would be aimed at solving this issue by letting you install one or several tools inside separate vendor dirs, so that:

  1. Tools have zero influence on each other.
  2. Tools have zero influence on the dependencies of your project, meaning you will always be able to upgrade your project's dependencies,
  3. In the case your tool has dependencies in common with your project, the tool will take your project's constraints into account.
  4. Tools packages might have a flag to opt-out of the above, so that if they never load the application code they can avoid being constrained by the application's dependencies.

The proposed section would not resolve the issue with tools that do require a version of a dep that is not compatible with your project. This is by design: as discussed in https://github.com/composer/composer/issues/5390#issuecomment-463625428, a phar could be the way to go for such tools.

A minimal example could look like the following:

"require-tools": {
    "phpstan": { "phpstan/phpstan": "^0.9.2" }
}

Some tools like Behat are extensible, and you would be able to install behat and its extensions like this:

"require-tools": {
    "behat": {
        "behat/behat": "^3.4.3",
        "behat/mink": "^1.7.1",
        "behat/mink-browserkit-driver": "^1.3.3",
        "behat/symfony2-extension": "^2.1.4"
    }
}

Of course, we could install phpstan next to behat like that:

"require-tools": {
    "phpstan": { "phpstan/phpstan": "^0.9.2" },
    "behat": {
        "behat/behat": "^3.4.3",
        "behat/mink": "^1.7.1",
        "behat/mink-browserkit-driver": "^1.3.3",
        "behat/symfony2-extension": "^2.1.4"
    }
}

The goal of these sections would be to create scoped vendor directories per entry in the require-tools sections. E.g. vendor/composer/tools/phpstan could contain all the deps of phpstan, etc.

Composer would also generate an autoload.php file in these scoped directories. Requiring this file would load two autoloaders: one for the vendor-dir of the project, and one for the vendor-dir of the tool.

Tools would always be considered as dev-deps, so that running composer i --no-dev would not install them (tools for prod could instead be added as direct deps of the project.)

When running composer u, composer would first resolve and install the deps as usual, then it would resolve the deps of each tool one by one, after locking the deps of the project.

The composer.lock file should be updated to store info about tool sections.

I'm sure there are more ramifications to this proposal.

Maybe this could be implemented as a Composer plugin btw, if anyone would like to give it a try?

danrot commented 3 years ago

Any reasons not to use Phive, which solves the problem by using a complete different tool for that kind of stuff?

Ocramius commented 3 years ago

What I do (and works fine) is having tools/<tool-name>/composer.json and tools/<tool-name>/composer.lock, then managed via @dependabot.

Makes everything much simpler, and treats the tool as a third-party as it should be, without any complex DSL for it.

canvural commented 3 years ago

Any reasons not to use Phive, which solves the problem by using a complete different tool for that kind of stuff?

Can you define your tool dependencies with Phive?

shyim commented 3 years ago

Any reasons not to use Phive, which solves the problem by using a complete different tool for that kind of stuff?

You will need a second installer.

Your suggestions seems to fit to https://github.com/bamarni/composer-bin-plugin

OskarStark commented 3 years ago

What I do (and works fine) is having tools//composer.json and tools//composer.lock, then managed via @dependabot.

Currently I am doing (nearly) the same (thanks to you @localheinz) and it works for me 👍 The thing I do different is, I have all my tools in just /tools/composer.json, but if I would ran into problems I could split them up 👍

greg0ire commented 3 years ago

Hey! I wrote this text in a private project we had in common but that I never had motivation to carry out, so let me address some of your remarks. Thanks to @nicolas-grekas for digging this up!

Your suggestions seems to fit to https://github.com/bamarni/composer-bin-plugin

Super cool tool, I set it up on https://github.com/doctrine/sql-formatter/pull/55 because I was getting stuck while trying to restore support for PHP 7.1. (too many constraints). It looks a lot like this suggestion with one key difference though:

In the case your tool has dependencies in common with your project, the tool will take your project's constraints into account.

This means the common dependencies would be reused (symlinked?) from the main vendor-dir to the tool's vendor dir. This ensures that there are no autoloading problems where loading one library in one version through the tool (the main example was symfony/yaml), you cause an issue with the code of your project because it expects another.

One may think that it still leaves some unnecessary constraint, but in my experience most issues come from constraints imposed on one tool by another , and not so much from constraints imposed by the project to the tool.

@Ocramius , the setup you use is cool, it's still prone to the autoloading issue though, and I'd say it might lack symlink from tools/<tool>/vendor/bin/ to vendor/bin. But maybe that autoloading issue is less common nowadays, I know at least PHPUnit has taken steps to alleviate it like not relying on symfony/yaml: https://github.com/sebastianbergmann/phpunit/commit/7a5b62c530a9d9cd29d36441defea93ef14a5722

All in all, it would be great to have something in the core for this, because that issue is common to both libraries and projects.

stof commented 3 years ago

https://github.com/bamarni/composer-bin-plugin has a slight difference (not counting the fact that it manages things with separate composer.json and composer.lock by wrapping normal composer executions in those folders): the tools it installs there have a totally separate set of dependencies, not taking the project dependencies into account (and so not conflicting either), which is great for tools which don't load your PHP code in their own process but still causes issues in case of a tool loading the project code (like PHPUnit)

Ocramius commented 3 years ago

@greg0ire if the tools share address space with the runtime application, they CANNOT be installed as tools, and MUST be part of require-dev.

stof commented 3 years ago

@Ocramius that's true for https://github.com/bamarni/composer-bin-plugin indeed. But the proposal of @nicolas-grekas allows (and enforces) to share address space between a tool section and the app (but not between multiple tool sections)

Ocramius commented 3 years ago

allows (and enforces) to share address space between a tool section and the app

I see that as a giant messy spaghetti nightmare, tbh.

greg0ire commented 3 years ago

I think that it would still be an improvement over the common practice that is putting every tool under require-dev which I know is a lot of pain in a lot of projects. There are alternatives, but they all involve either manual work, or another tool.

kevin-emo commented 3 years ago

I personally organize my tools in a very box shaped spirit :

tools/
    tool_1/
        config/
    tmp/
        vendor/
    composer.json
    composer.lock
    Makefile
    tool_2/
        ...
    Makefile

Pros :

Cons :

Nowadays, static linting tools like @phpstan, @psalm, @rectorphp, etc. are all sufficiently configurable to be able to consume external autoloads and work just as if they were part of the project's require-dev dependencies.

That's why I would be really happy to be able to scope my tools in one single composer.json file and fetch them apart from the project, and apart from each other aswell, in a single composer install command.

ondrejmirtes commented 3 years ago

I kind of dislike that PHPStan is used as an example of a tool that would use this feature, because I specifically spent a lot of effort on ensuring that PHPStan doesn’t need anything special from Composer to work properly as require-dev dependency and without version conflicts.

When you install phpstan/phpstan you get a package with a built PHAR file with zero external dependencies.

Other tools offer a similar option (like rector-prefixed or psalm-phar) but PHPStan pioneered the PHAR being the only option. By forcing it onto all users I was able to force myself fixing all the reported edge cases so it’s really solid now and works for everyone.

Even so that installing PHPStan with vendor-bin plugin is counterproductive and can break the analysis because of a different set of dependencies among other installed vendor-bin tools with versions that differ from the analysed app, which confuses PHPStan.

I’m not against the OP proposal but I think the “tool will take your project's constraints into account” part will only be possible with some additional work on the tool’s side. So this proposal presents additional work:

If tools adopted PHPStan’s way of installing a PHAR file, it’d only mean for them to learn working with Box and PHP-Scoper (and fix the unforeseen edge cases reported by users after the release, I admit), but they can do it today and Composer wouldn’t have to change at all.

(Phive isn’t an option, it doesn’t support version constraints between packages, like PHPStan + its extensions.)

greg0ire commented 3 years ago

I agree that the situation is far better now than it was when we initiated that project with a README looking like the proposal above. That was during the summer of 2018, and since then, things have changed for the better, most notably for PHPStan.

I’m not against the OP proposal but I think the “tool will take your project's constraints into account” part will only be possible with some additional work on the tool’s side.

What work are you referring to? IIRC "the tool will take your project's constraints into account" means that if the project has a package A installed, composer will reuse it in the tool's vendor directory (by creating a symlink from the main vendor dir to the tool's vendor dir for instance), and consider that package as locked when installing the tool. No work should be required on the tool's end, it would be fully on Composer's side (and yes, it might be hard to do).

ondrejmirtes commented 3 years ago

if the project has a package A installed, composer will reuse it in the tool's vendor directory

This would defeat the main reason why require-tools would be implemented - you want to be able to have package A in different versions used by the tool and in your project.

What I think "tool will take your project's constraints into account" means is that the tool should be able to query "Here in require-tools package A is in version 1.2 but in the project itself it's in version 2.4 - I'm interested in 2.4 for my purpose".

OskarStark commented 3 years ago

From my understanding it will have exactly the extra scope you mentioned, but in case they depend on the same version it will be shared, right?

greg0ire commented 3 years ago

This would defeat the main reason why require-tools would be implemented - you want to be able to have package A in different versions used by the tool and in your project.

Not entirely, you would still be freed from constraints imposed by one tool on another, and in my experience, that's where most of the pain comes (came?) from. For instance several tools relying on nikic/php-parser could cause issues, and it's unlikely to have that package required in the project.

From my understanding it will have exactly the extra scope you mentioned, but in case they depend on the same version it will be shared, right?

It's more that if the same package is found in the project, then it will be symlinked in the vendor-dir tool, and locked at whatever version it is in the project. That way the tool adapts to the project's constraints if any, but that should be a rare occurence.

ostrolucky commented 3 years ago

I would also like to mention current mess in IDEs that is caused by current workarounds in ecosystem for this missing composer feature. Here is a demonstration of it. It's really annoying

Screenshot 2021-01-26 at 11 22 15

In some cases it cannot even be excluded either

Screenshot 2021-01-26 at 11 27 19

I'm hoping such thing integrated in Composer would eliminate having to install same dependency multiple times when possible and hence, avoid having multiple misleading autocompletion entries. And I believe IDEs like PHPStorm would also start utilizing this feature and give you option to fine tune this, if it's integrated in composer.

theofidry commented 3 years ago

As a very long time user (and current maintainer although not much has changed for a long time) of https://github.com/bamarni/composer-bin-plugin, I think it fits the bill perfectly for a lot of use case and is quite transparent (it would be easy to ditch it to do it more "by hand".

One area of improvement though is the symlink generation for the binaries (see https://github.com/bamarni/composer-bin-plugin#what-happens-when-symlink-conflicts) which is the main reason why https://github.com/bamarni/composer-bin-plugin#disable-links exists (and I disable it every single time, and is not the default for BC). It feels that it would be much better if we had the bins in their respective vendors instead (e.g. vendor-bin/phpmetrics/vendor/bin/) instead of symlinking to the upper vendor bin directory (vendor/bin) or not having any symlink at all.

That being said, it doesn't address the last case @nicolas-grekas opened up which is for when the tool does autoload your code in which case you want the dependencies to be resolved. But I wonder, isn't that problem completely artificial? How this require-tools section be better than require-dev in this situation?

The only case that comes to my mind is when the tool in question is completely optional and code-independent. For instance unlike PHPUnit or Behat for which you write tests & contexts using a part of their API, a tool like infection could be used but because not enforced (for whatever reason) in your project, you don't want a dependency conflict to block everything in your project. In practice I solved this issue by combining https://github.com/bamarni/composer-bin-plugin & https://github.com/theofidry/composer-inheritance-plugin but this usage remains relatively rare.

stof commented 3 years ago

which is for when the tool does autoload your code in which case you want the dependencies to be resolved. But I wonder, isn't that problem completely artificial? How this require-tools section be better than require-dev in this situation?

@theofidry to me, the only benefit of the described require-tools over require-dev is when you have several independent tools which have this need but they have conflicting dependencies. Putting them in require-dev would result in a conflict. Putting them in separate sections in require-tools would work. For tools that don't need to autoload the project code, https://github.com/bamarni/composer-bin-plugin already fits the bill indeed (and I discovered that your readme links to https://github.com/theofidry/composer-inheritance-plugin to solve the case of needing compat with the main project dependencies)

ostrolucky commented 3 years ago

It's not the only benefit. Benefits I see:

stof commented 3 years ago
  • Composer being able to skip download/copy of same dependencies multiple times (I have yet to seen any solution which avoids duplicating all of those files)

the download part is already cached.

  • Autoloading (in case of no conflicts), so project could depend eg. on core Phpunit classes despite phpunit having conflicting dependencies with your project

I don't understand that. Your own code cannot access classes from tools, unless your own code is triggered by the tool code (in which case it is already the case with https://github.com/bamarni/composer-bin-plugin as well quite easily)

  • Solving IDE autocomple mess I pointed out in #9636 (comment), which is caused by having duplicate, auto-prefixed classes

I don't see how this relates to require-tools

Chi-teck commented 3 years ago

Tools would always be considered as dev-deps, so that running composer i --no-dev would not install them (tools for prod could instead be added as direct deps of the project.)

Tools for production may have same problems as dev tools and for the purpose of consistency this probably needs two sections require-tools and require-dev-tools.

"require-tools": {
    "behat": {
        "behat/behat": "^3.4.3",
        "behat/mink": "^1.7.1",
        "behat/mink-browserkit-driver": "^1.3.3",
        "behat/symfony2-extension": "^2.1.4"
    }
}

Why not just apply this format to require and require-dev sections? After all, anything in the vendor directory can be considered as a tool. composer require behat/mink --dev --group=behat

stof commented 3 years ago

Why not just apply this format to require and require-dev sections?

that would be a BC break for the ecosystem

After all, anything in the vendor directory can be considered as a tool.

anything that's accessed from your own code (rather than being a tool that your run that may then load your own code) is out of scope of that require-tools (otherwise, we would still have to register a single autoload, which forbids having separate sections)

Chi-teck commented 3 years ago

that would be a BC break for the ecosystem

Why? The groups are supposed to be optional and the installed package i.e. behat/mink would not be even aware that it is installed into the isolated group.

anything that's accessed from your own code

That's reasonable but using the term tool here is still confusing. It'll have to be expained in the documentation why phpstan is a tool but phpunit is not.

stof commented 3 years ago

@Chi-teck the structure of the composer.json (to add sections inside require) cannot change. That's why this issue suggests a new section

ostrolucky commented 3 years ago

the download part is already cached.

I wrote download/copy. They are not symlinked. It takes disk space, making eg. docker layers grow large and indexing via PhpStorm or search via grep longer. All completely unnecessarily.

I don't understand that. Your own code cannot access classes from tools, unless your own code is triggered by the tool code (in which case it is already the case with bamarni/composer-bin-plugin as well quite easily)

Why not? My code can access classes from tools if they are in normal require-dev directly. But perhaps I'm trying to bend this feature request more universal than was intended. This is better suited for libraries so indeed calling that tools wouldn't fit a bill. But my thinking here is that it should be doable to prefix only conflicting dependencies, so I could use libraries despite them having dependencies conflicting with my project.

I don't see how this relates to require-tools

I can either ignore whole vendor-bin or tool.phar and not get autocompletion when writing eg. phpunit tests, or not ignore it and get duplicate autocomplete results. This all results from all of these existing approaches not being aware of already existing, identical classes that they could reuse, but they don't, because they are completely isolated from each other with no softer levels of isolation possible.

stof commented 3 years ago

Why not? My code can access classes from tools if they are in normal require-dev directly.

That's the whole point. require-dev brings classes in your main autoloader. require-tools would not, which is precisely what would allow defining multiple sections which allow conflicting packages, as such conflicting packages would not be loaded together. require-tools is not about prefixing classes (which is very hard to do, as that changes the API of packages so you need to know what can be renamed and what cannot)

nicolas-grekas commented 3 years ago

Please have a look at #9792

glensc commented 3 years ago

I wish someone would have created GitHub actions workflow that:

  1. checks if box.json is present in the repo
  2. adds jobs that build phar "on: pull_requests"
  3. adds job "on: tags" to publish the phar as release asset

then perhaps adding .phar distribution to your project happens a lot more often.

OskarStark commented 3 years ago

@glensc I think it should not be a hurdle/requirement to create a PHAR to be able to be used by a project in a scoped way

Besides from this, your idea sounds interesting 😃 💡

mxr576 commented 3 years ago

I see conversation happens about the same topic in several places :) Let me reference my comment from an other one: https://github.com/composer/composer/pull/9792#issuecomment-807983199

to create a PHAR to be able to be used by a project in a scoped way

How about bundling dev tools in scoped PHARs as a solution? https://github.com/humbug/php-scoper + https://github.com/box-project/box

stof commented 3 years ago

@mxr576 scoping a phar with php-scoper requires that project maintainers do it, to configure which classes can be scoped and which one must not be renamed because they are necessary as a public API (for instance, if you scope PHPUnit, renaming the TestCase class would break everything). So that cannot be a solution implemented by composer directly. Projects wanting to provide scoped phars can still do that, even if other solutions are available to support projects not doing it.

mxr576 commented 2 years ago

Has anything related happened outside of this thread in the past year?

internalsystemerror commented 1 year ago

Every now and again I keep coming back to this issue for one reason or another. I like this current proposal, however it still wouldn't solve the issue where you use one dependency (say PHPStan or ECS) to lint/analyse another dependencies configuration (say ECS or Composed-Unused, both of which use a PHP configuration file).

None of the currently available methods (manual tools/* dir, composer-bin-plugin, phive) really solves that well. I still end up having to add phpstan and/or ECS to each of the tools dependency trees (e.g. phpstan to ecs, phpstan AND ecs to composer-unused etc) where that's even possible. That said, a consistent method would be preferable.