-
Notifications
You must be signed in to change notification settings - Fork 430
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
Consider adding a composer.lock
file in the repository
#1095
Comments
There are lot of downsides of having composer.lock. Including composer.lock is internal decision for development process. Missing it is often easier because it doesn't cause conflicts, bloat diffs, commits or repo size and ensures always latest dependencies are installed. As for distribution, grumphp is distributing .phar file, that achieves your reproducibility goal of Nix and hence there is no need for composer.lock, is there? As for cherry picked examples, I can provide counter examples as well. We removed composer.lock from Doctrine projects, because it was causing lot of problems. Especially if project needs to support multiple PHP versions and each version needs its own composer.lock file, this is very difficult to solve. And having it means all the PRs touching it are asking for conflicts that need to be resolved once any of them is merged. |
While there can be downsides to including a
Yes, Grumphp is distributing a A PHAR file, while handy, does not offer transparency in its build process (there are existing cases!). Unless you manually inspect the PHAR, there is no way to ensure its security or the exact versions of dependencies it includes. Therefore, including a Including a |
@drupol Just a little brainspark : Since I am compiling the PHAR with a locked set of dependencies (based on lowest available PHP version), would it make sense to add that lock file, that is being created during PHAR generation, as an upload to the release (just like I provide the PHAR there)? I could also make sure it gets commited to the shim repository. I am running some basic checks during deploy and PHAR generation, but I am not soing an in-depth security scan of the projects dependencies at the moment. I trust packages like doctrine and symfony to be well managed and security updates to be present in latest possible releases of those projects. The arguments of @ostrolucky is what is keeping me back from adding the lock file to the codebase: Besides the different PHP versions which might require different dependencies, it also breaks the min / max dependencies checks that are built-in to the CI pipeline, since the dependencies are being locked. When I notice something breaks either in the min or mex dependency range, I am able to resolve it quite fast - because there is no lock file. Don't get me wrong: |
I understand your concerns about the additional overhead and the complexities associated with managing the Regarding the issue of supporting multiple PHP versions, it's true that managing a single As for the min/max dependency checks, while locking dependencies does seem to interfere with this mechanism, it is also a way to ensure that the software is tested against known-good versions of its dependencies. We can keep the process of updating dependencies controlled and deliberate, rather than allowing it to occur arbitrarily based on when a developer or the CI pipeline happens to run I fully understand the fear of dependency hell, but the reality is that the Of course, finding a middle ground is the key here. Perhaps we could consider committing There's no "one-size-fits-all" solution here, and the best approach might depend on the specifics of your project and team. In fact, a viable middle ground could be to follow the approach adopted by the Composer project itself. They include This approach can provide us the best of both worlds - it helps ensure a controlled, consistent development environment while also allowing for flexibility in dependency management when the project is used as a dependency itself. It can offer a solution that balances reproducibility, security, and maintainability. What are your thoughts on this? |
Can you link me to one or some of those repositories so that I can take a look on how they manage this?
Now this is actually a thing I think of as a problem: Most of our installs (7,5m) are coming through the regular package's composer installation process. The shim version has a bit more than 1m installs. Stats:
Since both are being installed through composer, it fit me no purpose to lock the dependencies during development : Since we allow PHP from 74 'til 8.2 various versions of GrumPHP (v1) and on a range of symfony components from 5.4 to 6.4 - I do want to get as much feedback as possible during development. I want to know if it crashes on the combination Symfony 5.4 - PHP 74 - composer1, because this feedback allows me to make sure this keeps on working on pretty much all possible supported combinations. This allows other developers than me to report and/or fix these kind of bugs for me. If I were to lock the dependencies down, they won't be able to reproduce nor fix the issue. (This was for example a very recent case in a bug that got introduced in amp/parallel a few weeks back: I wouldn't be able to detect this with locked dependencies) Reproducibility here is a non-existing, since every project has its own set of (locked) dependencies.
Finding a middle ground is exactly what I am attempting to figure out with this RFC. All opinions and approached are very much welcome!
Then we should at least:
on every PR / commit, on every platform, on every PHP version + introduce some auto-update lock file mechanism that is somehow PHP version aware. Not sure if this exists already somewhere? If you know of some, can you point me to a package that does this? Again all very good points you make and I'm really in between visions here - but on the other hand, I can only add some kind of locking mechanism if it isn't a burdon for me to maintain and if it still results in us being able to fix dependency issues in a rather broad set of supported 3rd party packages on very short notice. |
I must admit, finding a concrete example of a project maintaining separate As for the concerns you're raising, I think I understand where they're coming from. However, I believe the key difference here lies in the nature of the project. When Considering this, the strategy I suggested earlier still stands as a possible solution:
By doing this, we maintain the I hope this clears up some confusion. Let's continue discussing to find the best approach and also, feel free to let me know if I missed anything or if I'm totally wrong. Thanks. |
The suggested way of composer's installation is by downloading the binary directly from the internet. Another project you mentioned before is phpunit. Yet whilst searching their repository, I did not encounter any composer.lock file. In contrary: the build process explicetly removes it. Even though the suggested way is installation from their online binary.
That is indeed how lock files work. The lock file would only be used for 1. people who want to hack on GrumPHP - or 2. by the nix build process you introduces in order to guard for reproducibility. As mentioned in previous comment, I don't really want option 1 to be true : I want them to work with latest or lowest dependencies so that we can detect issues early. Can we agree I'll have to go through a lot of pain for this 1 sole (at the moment rather limited) purposes? That's why I suggested of shipping the lock file as part of the phar release generation process and link it as an attachment to the release instead in the first place: at the moment of phar generation, I have a lock file that works on minimum supported PHP version. There is no guarantuee though that it is guarded against security risks atm and that it works an ALL PHP versions (because of dependencies). Yet, with current set of dependencies - it should to the trick. |
Discussed in https://discord.com/channels/1090672946370052166/1091081297759309926/1121038709295435846 with @veewee The problem of multiple PHP versions and one
|
Just some small side-marks from my point of view:
It's a bit more complicated
That's what we basically do right now for building the phar distribution: It installs the dependencies for lowest PHP version and assumes it works for upper PHP versions as well. The github actions does a very shallow check if this works. |
"Support" as in "fix bugs": do you really care about anything non-bug merged up? I wouldn't even introduce
From what you say here, you already have what you'd call a "committed
Yeah, I don't like I'd say that what @drupol will potentially be able to provide is both better and more stable at the same time. |
@drupol taking the whole discussion into consideration, I believe this PR is the best thing I can do for you: Together with the Your nix flake would now need to:
I don't think this will change much to your current process, since you already point to an external lock file. Can you find yourself in this solution? |
@veewee @Ocramius thank you so much for your valuable inputs. I have been thinking about this thread back and forth. Since Grumphp act as both a library and a binary, finding a balance for these distinct use-cases can indeed be challenging 😅 As such, I believe the PR you're proposing provides a sufficient solution for now. While it may not be my favorite choice, it does contribute towards the goal of having the The reason of all of this is because I'm working on quite a pull request within Nix, and I've been working on it for the past 2 months. Right now, making sure a Composer-based application can be "reproducable" in Nix is not so straightforward. Having a A collateral bonus feature of this work is that all the projects distributed in Nix via a PHAR file will be replaced by their original source code counterpart. This won't change anything for the end user. There are lots of PHP projects used as binaries that don't have a Grumphp is the first project where I ask such a thing, but given the valuable inputs I received it, I will likely refine my approach and my request for the next project. |
Nice, glad we agreed on a common ground and can continue collaborating in building amazing tools for the community. I've merged the PR and it will all work starting from the next release in the v2 range of GrumPHP. For now the lock file is already available in the branch of the grumphp-shim repo.
Your work on nix looks amazing! I wish you good luck in persuading other packages as well. |
FYI, I just updated the Grumphp derivation, including YOUR Issue closed for me as well. |
Hi,
I'm opening this PR to try to get the file
composer.lock
live in the repository and be part of the Grumphp sources.Summary
The
composer.lock
file, though sometimes overlooked, is key for project reproducibility, performance, and security.Rationale
Composer is the dependency manager for PHP. It allows you to declare the libraries your project depends on and it will manage (install/update) them for you. In a
composer.json
file, you specify the dependencies for your project. For example, you might need a certain library, and that library might need another library, and so on.When you run
composer install
for the first time, it resolves dependencies and then downloads all of them and writes all of the packages and the exact versions of them that it downloaded to thecomposer.lock
file, locking the project to those specific versions.Committing this file in this repo is important for many reasons:
Reproducibility: Reproducibility is one of the foundational aspects of software engineering. It ensures that all team members and the deployment system are using the exact same versions of the dependencies. This eliminates the potential issues that may arise from differences in versions used, such as function deprecations, changes in behavior, etc. Without the
composer.lock
file, runningcomposer install
would fetch the latest versions of the dependencies you defined incomposer.json
, which could cause inconsistencies between environments.Security: It helps ensure that you're using the versions of dependencies that you have tested and reviewed. Without a
composer.lock
file, there's a risk that a malicious person could find a vulnerability in one of your dependencies, fix that vulnerability, bump the version number, and if you runcomposer install
orcomposer update
, you could be pulling in un-reviewed/un-tested code since you do not control anymore the dependency resolution and delegate it to the host system.Speed: Having a
composer.lock
file speeds up the composer installation process. Composer no longer needs to resolve dependencies (which can take some time) and can just download the version of each dependency specified in thecomposer.lock
.Inclusion of
composer.lock
in this repository also promotes the following advantages (but I won't go into the details, this post is already too long):Conclusion
While the
composer.lock
file may appear nonessential and is sometimes overlooked, its inclusion is critical for ensuring the reproducibility, speed, and security of your project. By including thecomposer.lock
file in your project's repository, you are effectively shifting the responsibility of specifying correct dependencies from the end-users to the project's author.Consequently, the author takes charge of determining the precise dependencies to install, sparing the users from this task. This approach streamlines the installation process and fosters a consistent project environment.
By including the
composer.lock
file in the repository, not only does it improve the development experience for contributors, but it also provides users with an assurance of quality, security, and reliability. The presence of thecomposer.lock
file signifies that the project's dependencies are carefully managed and tested. It confirms that the project operates effectively with the locked versions of dependencies, thus reducing the risk of compatibility issues, unexpected behavior and bugs.The
composer.lock
file ultimately serves as a testament to the project's stability. It safeguards users against potential risks associated with untested updates or patches in dependencies. Therefore, both contributors and users can benefit from the project author's meticulous dependency management reflected in the committedcomposer.lock
file. This can enhance the overall confidence in the project's robustness and reliability.Further Considerations
While Grumphp is developed in PHP, it can also be viewed as a binary, not solely a PHP library. Given this, it's imperative that it retains reproducibility across various environments.
composer.lock
is our most effective asset to ensure this within the PHP ecosystem. Considering the reasons outlined here, I strongly believe it deserves a place in the project.What others are doing ?
It's often beneficial to consider practices in other projects. Composer, the PHP package manager itself, follows this practice. There are many more such examples in the ecosystem.
Application, binaries, libraries
A PHP application or binary is a standalone piece of software intended to be installed and run as is, and the
composer.lock
file should be included. This is because the application or binary needs to ensure that it is using the exact versions of its dependencies that it was developed and tested with. If the dependencies were to change unpredictably, the application could break. Therefore, thecomposer.lock
file, which specifies the exact versions of the dependencies, is essential.On the other hand, a PHP library is a piece of software intended to be included as a dependency of other projects. In this case, the
composer.lock
file is typically not included in version control. This is because the library should be compatible with a range of versions of its own dependencies. The library doesn't control the exact versions of its dependencies - the parent project that uses the library does. As such, the dependency resolving mechanism should indeed be handled by the project that requires the library, not the library itself.Relation with Nix
Reproducibility is a hard requirement when bundling packages in Nix. To build a reproducible version of Grumphp, we need to ensure all inputs are accurately provided to Composer. We currently provide the
composer.lock
ourselves to ensure consistent use of the same dependencies, and I believe we should not do that. Making developers aware of the significance of this file would be beneficial.Practical, hands on
To ensure reproducibility with a Grumphp build, containing
composer.lock
:git clone https://github.com/phpro/grumphp
cd grumphp
This next step is mandatory to prevent Composer to generate random class names for the Autoloader:
composer config autoloader-suffix FooBar
Let's install the required dependencies only:
composer install --no-dev
The next command will compute a checksum of the
grumphp
directory containing now thevendor
directory:cd -; dir=grumphp find "$dir" -type f -not -path '*/\.git/*' -exec sha256sum {} \; | LC_ALL=C sort -d | sha256sum; cd -
When the
composer.lock
is available, there is a guarantee of reproducibility, you could run this on any platform and you will always get the same checksum.When the
composer.lock
is not available, the checksum might be different from a day to another, there is absolutely no guarantee of reproducibility.The text was updated successfully, but these errors were encountered: