-
Notifications
You must be signed in to change notification settings - Fork 49
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
Adding composer.lock
in VCS ?
#95
Comments
Hi,
As already explain in some of the other issues you created, adding a composer.lock file in the repo of a composer package is more a hindrance than a benefit (i would even say it's a bad practice). Contrary to a production project (e.g. a drupal or magento website), a php package is compatible with multiple php versions, and there is no need to have a development environment that matches a production environment. Not commiting the lock file ensures that the vendor dir always matches the version of the PHP runtime (e.g. for people who work on a PR, or for a CI pipeline that runs unit/functional tests, such as https://github.com/Smile-SA/gdpr-dump/blob/main/.github/workflows/tests.yaml).
Currently it would not be enough. If you want to create exact copies of our released phars, you have to make the same actions as our github workflow that creates the releases (notably the update of the constant the stores the app version, cf. https://github.com/Smile-SA/gdpr-dump/blob/main/.github/workflows/release.yaml#L45). To keep things as simple as possible, this change is not committed anywhere. We can probably solve this issue by using the vendor/composer/installed.php file to determine the version (I think it's what phpstan does).
It would not be complicated to implement (just an additional step in the github workflow), but I fear that a lot of people would be confused by this ("What the hell is this file? What do I use it for?" 😅). |
I fully understand. I wouldn't say it's bad practice, but I understand that it might be a change of habits and therefore be considered as a hindrance.
I can attest that this is really enough to make it reproducible, and this PR is actually the proof (NixOS/nixpkgs#286809). Nix will create identical copies of
As a middle ground solution, would you be OK to publish the |
I meant to say adding a composer.lock file is not enough, as you also need to update the app version before creating the phar. But it's now a thing of the past, I just pushed a commit that automatically detects the version (same logic as phpstan).
As said in my previous message, I fear that it could confuse users who want to download the phar and don't know what this file is for. Do you really need this file? Not using a lock file would ensure that your phar file has the most up-to-date dependencies (which is good for security). |
And what I was saying is that that particular step is optional since I was already able to get something reproducible.
This commit assumes multiple things to be true now. It expects that the Here's a script example where this would fail (the rm -rf main.zip gdpr-dump-main
echo "Getting a snapshot of the main branch..."
wget -q https://github.com/Smile-SA/gdpr-dump/archive/refs/heads/main.zip
echo "Unzipping..."
unzip -qq main.zip
cd gdpr-dump-main
echo "Running composer install..."
composer install --no-dev --quiet
echo "Compiling the PHAR..."
php -d phar.readonly=0 bin/compile
echo "Print a PHAR checksum..."
sha256sum build/dist/gdpr-dump.phar
echo "Check the PHAR version..."
php build/dist/gdpr-dump.phar --version
Sad thing, I'll close both issues then. |
Hi there,
I'm writing to suggest an enhancement for this project to improve its integration and ensure long-term reliability, particularly for package managers like apt, rpm, yum, nix, etc etc.
The primary goal here is to provide full reproducibility of the exact build of the PHAR released, down to the last bit. This is crucial, especially when distributing it as a binary for reasons of reliability and security.
For example, without
composer.lock
, running twocomposer install
at different date and time could potentially create 2 differentvendor
directories. This is what we strive to avoid at all cost.To achieve full reproducibility, I propose adding a
composer.lock
file to this repository. This file will ensure thatgdpr-dump
is reproducible on any system, regardless of environmental differences, by locking down specific versions of dependencies. Its absence currently hinders the ability to precisely replicate the builds you provide in your GitHub releases, introducing potential variability.For example, in Nix, we have to provide our own
composer.lock
for each update, as seen in this PR: NixOS/nixpkgs#286809 (I'll keep that PR in draft until I have some feedback about this issue).Having a
composer.lock
file in your responsibility would transfer the responsibility of providing something stable and reproducible to this project instead of delegating it to anyone else.For reference, here are some projects that have recognized the value of versioning the
composer.lock
file:composer.lock
file in the repository bobthecow/psysh#767composer.lock
file in the repository phpro/grumphp#1095If for any reason there are concerns about including the
composer.lock
file in the main repository, could it be an option to add it to the release artefacts on GitHub along with the PHAR file? This middle-ground compromise would still greatly benefit reproducibility and consistency in dependency management.Thank you for considering this enhancement. I look forward to your response.
The text was updated successfully, but these errors were encountered: