Composer.lock is commited

[sagikazarmark]

I'm trying to understand why the composer.lock file is committed.

The change was made by @Ocramius here: https://github.com/moneyphp/money/commit/0d1ec59efd43e66d2f3d671eb390d110055944f2

The reason being that it will lock the versions for tools like Psalm, PHPCS and PHPStan.

My questions are:

• Why not just pin the versions in composer.json?
• Why not use the composer-bin-plugin for these dependencies?

My primary concern (apart from the potential git conflicts) is that people will start bumping dependencies and it will get much harder to review what changed compared to a single version update or psalm dedicated composer.json/lock.

@Ocramius @frederikbosch any thoughts?

[frederikbosch]

I prefer not to commit composer.lock but that is mostly based on arguments that were valid years ago. In defense of not committing it, I would think - but @Ocramius might reject my argument for valid reasons - that not committing composer.lock leaves contributors with the problem of creating a valid PR that passes all checks. And when a PR results in an error with tools like Psalm, PHPCS and PHPStan due to a version upgrade, that it is up to the contributor to solve it. The contributor can choose to narrow a version, or to fix the issue with the tool at hand.

Maybe @Ocramius is willing to explain his reasons, or to link to a blog/article/text that contains the reason.

[Ocramius]

I explained this in more detail at https://github.com/mariosimao/notion-sdk-php/pull/6#discussion_r744177586

Quoting:

Yes, locked means using a committed composer.lock.

You can put your composer.lock in export-ignore, and it will be ignored regardless by downstream composer install and composer update operations.

The reasons why composer.lock is valuable is that tools like vimeo/psalm, phpunit/phpunit and phpcs/php_codesniffer often contain bugfixes and tiny improvements that completely destabilize the whole downstream consumer CI pipelines. This is also not fixable on their end: if vimeo/psalm slightly improves the type inference of an internal PHP function, for example, you might get reported RedundantCondition or similar errors in your pipeline.

By running them locked, you can come back to this project one year from now:

• latest pipeline will have "discovered" any changes that might be problematic
• lowest pipeline might highlight how some dependencies no longer work with your code changes (signatures, mostly)
• locked will allow you and any contributor to just clone the repo, run composer install and start working without worrying too much about unrelated failures.

I therefore encourage to run these specific tasks only with locked:

• coverage
• static analysis
• mutation tests
• code style checks

For phpunit itself, you can run latest + lowest + locked.

Important for this to work is to set up @dependabot for your project. See for example:

• https://github.com/Roave/BackwardCompatibilityCheck/pull/320/files#diff-dd4fbda47e51f1e35defb9275a9cd9c212ecde0b870cba89ddaaae65c5f3cd28
• https://github.com/Roave/BackwardCompatibilityCheck/pull/320/files#diff-82f8802c05df24bb863565fa5512918da7cdced6a3395798ed3afbe58ca9c230

Therefore:

• good to commit composer.lock
• good to have @dependabot look at it
• good to run latest and lowest dependencies tests, but NOT for phpcs, phpstan, psalm, etc (too fragile)
• run phpcs, phpstan, psalm, etc with locked dependencies

See also https://github.com/laminas/technical-steering-committee/blob/main/meetings/minutes/2021-08-02-TSC-Minutes.md#locked-dependencies-for-all-supported-php-versions

[sagikazarmark]

that not committing composer.lock leaves contributors with the problem of creating a valid PR that passes all checks

That's true regardless of commiting the lock file (ie. the CI runs composer update to make sure we support both the lowest and the latest dependencies)

And when a PR results in an error with tools like Psalm, PHPCS and PHPStan due to a version upgrade, that it is up to the contributor to solve it

That's where the composer-bin-plugin can help (see Guzzle for example).

[Ocramius]

Note: I used to be against committing composer.lock, but I changed opinion over the years, when having to deal with the instability of many of these tools, which is inherent of their internal improvements.

A super-stable build aids contributors and maintainers in isolating bugs.

[Ocramius]

That's where the composer-bin-plugin can help (see Guzzle for example).

Composer plugins are generally something to always avoid.

[sagikazarmark]

I see. Well, I can't say I'm fully convinced, even though I see the problem that committing the lock is trying to address.

Composer plugins are generally something to always avoid.

Can you elaborate? Do you have any references that I could read?

[frederikbosch]

@sagikazarmark I do agree here with @Ocramius. Also in this library, we have invested much more time in updating tools than in fixing bugs or creating features for the lib itself. Anything we can do to prevent us from that type of work is something should embrace that.

[Ocramius]

Can you elaborate? Do you have any references that I could read?

Reference: me.

As a composer plugin author, I can tell you that most composer plugins destabilize the whole download, install and upgrade operations, as well as the dependency graph, enough to make a build not reproducible, and therefore invalid.

Your dependencies should be:

• in a composer.json + composer.lock
• in a Dockerfile (although still unstable due to base layers)
• in a .nix derivation (extremely locked)

Relying on how your composer binary has been manipulated ("tampered with") is not viable.

[sagikazarmark]

Thank you both for your comments!