-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add require-tools section and handle scoped vendor dirs #9636
Comments
Any reasons not to use Phive, which solves the problem by using a complete different tool for that kind of stuff? |
What I do (and works fine) is having Makes everything much simpler, and treats the tool as a third-party as it should be, without any complex DSL for it. |
Can you define your tool dependencies with Phive? |
You will need a second installer. Your suggestions seems to fit to https://github.com/bamarni/composer-bin-plugin |
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 |
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!
Super cool tool, I set it up on doctrine/sql-formatter#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:
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 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 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. |
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) |
@greg0ire if the tools share address space with the runtime application, they CANNOT be installed as tools, and MUST be part of |
@Ocramius that's true for https://github.com/bamarni/composer-bin-plugin indeed. |
I see that as a giant messy spaghetti nightmare, tbh. |
I think that it would still be an improvement over the common practice that is putting every tool under |
I personally organize my tools in a very box shaped spirit :
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 That's why I would be really happy to be able to scope my tools in one single |
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 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.) |
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.
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). |
This would defeat the main reason why 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". |
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? |
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
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. |
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 In some cases it cannot even be excluded either 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. |
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. 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 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. |
@theofidry to me, the only benefit of the described |
It's not the only benefit. Benefits I see:
|
the download part is already cached.
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)
I don't see how this relates to |
Tools for production may have same problems as dev tools and for the purpose of consistency this probably needs two sections
Why not just apply this format to |
that would be a BC break for the ecosystem
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 |
Why? The groups are supposed to be optional and the installed package i.e.
That's reasonable but using the term |
@Chi-teck the structure of the composer.json (to add sections inside |
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.
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
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. |
That's the whole point. |
Please have a look at #9792 |
I wish someone would have created GitHub actions workflow that:
then perhaps adding .phar distribution to your project happens a lot more often. |
@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 😃 💡 |
I see conversation happens about the same topic in several places :) Let me reference my comment from an other one:
How about bundling dev tools in scoped PHARs as a solution? https://github.com/humbug/php-scoper + https://github.com/box-project/box |
@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. |
Has anything related happened outside of this thread in the past year? |
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. |
This issue is a follow up of #5390 (comment)
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, calledrequire-tools
, that would be aimed at solving this issue by letting you install one or several tools inside separate vendor dirs, so that:meaning you will always be able to upgrade your project's dependencies,
tool will take your project's constraints into account.
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 #5390 (comment), a phar could be the way to go for such tools.
A minimal example could look like the following:
Some tools like Behat are extensible, and you would be able to install behat and its extensions like this:
Of course, we could install phpstan next to behat like that:
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?
The text was updated successfully, but these errors were encountered: