Skip to content
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

chore: fix phar.composer.lock #24

Closed
wants to merge 1 commit into from
Closed

chore: fix phar.composer.lock #24

wants to merge 1 commit into from

Conversation

drupol
Copy link

@drupol drupol commented Oct 21, 2023

Hello,

This PR relate to #23 and fix the composer.lock validation. There's no more discrepancies between composer.json and composer.lock now.

Built with PHP 8.2.11 and Composer 2.6.5

Built with php-8.2.11
@veewee
Copy link
Collaborator

veewee commented Oct 21, 2023

The file is (and should be) autogenerated.
As mentioned in the issue, I think the issue there is that the composer.json of the grumphp directory you are building should contain the configuration for php 8.1:

composer config platform.php 8.1'

I don't think the plugin-api-version will cause issues, cause it's the version of composer that built the lock file.
But if that's an issue, we'll have to pin the composer version somehow during builds (which I'm not really looking forward to).

@veewee veewee closed this Oct 21, 2023
@drupol
Copy link
Author

drupol commented Oct 21, 2023

Ok will try with that.

@drupol drupol deleted the fix-composer-lock branch October 21, 2023 09:48
@drupol
Copy link
Author

drupol commented Oct 21, 2023

Looks like it's still creating issues.

❯ cp phar.composer.lock composer.lock
❯ composer config platform.php 8.1
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ❯ php ^C                                                                                                                   192.168.2.188 |  | ❄
❯ composer validate
./composer.json is valid but your composer.lock has some errors
# Lock file errors
- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`.
❯ composer update --lock
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
75 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> GrumPHP\Composer\DevelopmentIntegrator::integrate
Watch out! GrumPHP is sniffing your commits!

Found 1 security vulnerability advisory affecting 1 package.
Run "composer audit" for a full list of advisories.
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ❯ cp ^C                                                                                                          192.168.2.188 |  | 1s245ms | ❄
❯ diff phar.composer.lock composer.lock
7c7
<     "content-hash": "8a069c630e6ddbc4475db9a992430539",
---
>     "content-hash": "0474062650b24a22c63007631cf35f1e",
7093c7093
<     "plugin-api-version": "2.3.0"
---
>     "plugin-api-version": "2.6.0"
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ✘  

But OK, I'm a bit out of ideas on how to get this things working right now, Grumphp 2 will wait a bit before being packaged in Nix. I pushed this PR, we'll see what the other PHP maintainers will think.

@veewee
Copy link
Collaborator

veewee commented Oct 21, 2023

As you can see in the logs, only the composer API version differs now. Meaning you'll have to use the exact same composer version to validate the file that infused to build the PHAR or somehow disable that specific line during the validate command.

Not sure either how we can do this tbh

@drupol
Copy link
Author

drupol commented Oct 21, 2023

I could also just wait for the next point release, hopefully your composer version will be updated :D

@veewee
Copy link
Collaborator

veewee commented Oct 21, 2023

It might or it might not be :)
In any case, this way it won't be reproducible in time either. Can imagine you'll bump into similar issues on other packages you are trying to ship as well.

It might make sense to ask the composer people how to deal with it?

@drupol
Copy link
Author

drupol commented Oct 21, 2023

It might or it might not be :)

OK ! But what's preventing you to automatize generation of the composer.lock using the latest Composer version in your workflow?

We are always trying to use the latest stable version of Composer, we cannot use an arbitrary version of composer and/or maintain all the versions in the repository. This is not only specific to Nix, but in every distributions. Composer being the most important tool here.

In any case, this way it won't be reproducible in time either. Can imagine you'll bump into similar issues on other packages you are trying to ship as well.

So far this problem only happen with Grumphp. I doubt there will be similar issues with others... (yet?).

I could fix the issue by providing patches, but I would have preferred fixing the problem within this repo than polluting nixpkgs with patches that would be removed at some point.

@veewee
Copy link
Collaborator

veewee commented Oct 21, 2023

I get that.
From my point of view , there is nothing wrong with the provided lock file though.

I could upgrade composer before building the phar, but if you build your package a week later - and a new composer version bumps the API version in the lock file, your build will fail again.

I'm rather suggesting looking into a more stable build process from your side, since that's the more time-proof solution.

@drupol
Copy link
Author

drupol commented Oct 21, 2023

From my point of view , there is nothing wrong with the provided lock file though.

True if you use a version of Composer < 2.6.0. With the current version of Composer, the validation is failing. So to me, there's an issue with older versions of composer. Of course this is not critical, and we can still run composer install it will work, but in automated processes where we would like to integrate a composer validate in between, this is an issue. That's the reason why I started to work on a PR where we could skip this validation process at NixOS/nixpkgs#262388.

I could upgrade composer before building the phar, but if you build your package a week later - and a new composer version bumps the API version in the lock file, your build will fail again.

No, we can use different Composer version, but if the attribute composer-plugin-api change we will have issues.
It has been changed 4 months ago, at the release of Composer 2.6.0...

But allez, no worries, I will just wait an update of Grumphp and in the meantime still use 1.15 or just provide my own composer.lock.

As always, thanks for your availability, even if we haven't found the best compromise.

@drupol
Copy link
Author

drupol commented Oct 21, 2023

I fixed the issue in Nix by providing 2 very small patches, find it here: NixOS/nixpkgs#262558

@veewee
Copy link
Collaborator

veewee commented Oct 21, 2023

Allright nice.

In any case, I'll update the script in order to download the latest composer before building.
I doubt it will solve all issues in the long run, but we'll see what kind of lemons life will throw at us afterwards :)

Probably something for next friday. I'll keep you posted.

@drupol
Copy link
Author

drupol commented Oct 21, 2023

Thank you mate, ping me when you need me to check it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants