-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Composer/Packagist: support platform compatibility during lookup #2355
Comments
@swashata do you know if there’s a standardised field in packaging responses that indicates each version’s compatibility? |
I have already checked and it seems there isn't a standardized field. Actually if we are to publish a package that requires some particular version of PHP, we put it under A good example is this https://packagist.org/packages/paragonie/random_compat.json For different versions of the package, there are I will do some more digging and will let you know. |
Also do note that when defining platform (in the app), we don't specify PHP versions like {
"config": {
"platform": {
"php": "5.6.0"
}
}
} It is upto the libraries that app uses to make sure they |
In my
Maybe this can be used? |
hi, |
@rarkins what are you willing to accept to see this fixed? This is causing me an issue where: My composer:
Endroid composer:
The
I'm not sure why renovate is updating |
Renovate The lack of |
@rarkins I think this should be simpler than expected to implement.
Example commands: If someone points me in the right direction then I can submit a PR but this feels like it's achievable very quickly. At a complete guess (I don't know TypeScript) so it will take me significantly longer than someone who does: // extract.ts
if (composerJson.platform) {
res.compatibility = composerJson.platform;
}
// artifacts.ts
if (config.compatibility) {
for (const [depName, version] of Object.entries(config.compatibility)) {
args = 'config platform.' + depName + ' ' + version;
logger.debug({ cmd, args }, 'composer command');
await exec(`${cmd} ${args}`, execOptions);
}
} |
@bytestream Why do we need to set the config options to composer? I think extracting the Does composer have problems with different php patch releases, because our php docker images currently only allows to build latest patch for supported php minor versions. we are using https://deb.sury.org/ as php source |
I don't fully understand how renovate bot works but it ignores the
No. I don't fully understand what you mean with this question. |
@bytestream My questions was simply can we use php 7.2.6 if the composer platform config says 7.2.3? So using a higher php patch release to update the lockfile. |
Yes, anything greater than the specified platform version.
Sent via mobile.
Please excuse the spelling, brevity and punctuation.
…On Tue, 9 Jun 2020, 17:03 Michael Kriese, ***@***.***> wrote:
@bytestream <https://github.com/bytestream> My questions was simply can
we use php 7.2.6 if the composer platform config says 7.2.3? So using a
higher php patch release to update the lockfile.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2355 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANUT3J3LLK7AZDHK25OE4LRVZMLLANCNFSM4FOIKJXQ>
.
|
So how do you think this should be implemented @viceice ? I'm keen to get movement because my previous fix is not feasible when using renovate bot on multiple repositories with different platform requirements. |
first we need to add before here: renovate/lib/manager/composer/extract.ts Line 185 in aed8d75
if (composerJson.platform) {
res.compatibility = composerJson.platform;
} then we need to add renovate/lib/manager/composer/artifacts.ts Lines 112 to 114 in aed8d75
like in renovate/lib/manager/bundler/artifacts.ts Lines 134 to 138 in aed8d75
|
Thanks for the merge! In order to ensure that renovate runs using a specific PHP version, you have several options:
We are currently using the second option to specify a specific version as we use a wide constrain for the first. The PR referenced will ensure that updates to the |
How are you using the second? Because currently I'm attempting to do that and it still seems to take the highest available version from the first when resolving the possible package version for a bump. |
Are you specifying the full version? Here is an example of how we're using it: https://github.com/laminas/laminas-servicemanager/blob/f22969b8b0ea2d695aa832c8895be29622b9bd94/composer.json#L25 "config": {
"platform": {
"php": "8.0.99"
}
} This is treated by packagist as essentially |
Yup using the full version, and got it in every |
This comment was marked as resolved.
This comment was marked as resolved.
Hi there, Get your issue fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible. Before we start working on your issue we need to know exactly what's causing the current behavior. A minimal reproduction helps us with this. To get started, please read our guide on creating a minimal reproduction. We may close the issue if you, or someone else, haven't created a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment. Good luck, The Renovate team |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
After reading several issues and trying different variants, I'm leaving the below comment as a summary of my findings. As of https://github.com/renovatebot/renovate/releases/tag/34.154.4, this is how it works for Drupal projects. About platform version constraintsRenovateBot assesses 3 places to determine the PHP platform requirements:
The purpose of The purpose of The purpose of Which versions to specify in a Drupal site?It is recommended to specify It is also recommended to specify Once the PHP version of your environment is updated, the It is not recommended to use CaveatsIf Working exampleI have setup a Drupal-related repo (and been supporting it) to provide zero-config Renovate setup for Drupal projects (GitHub app and self-hosted in GitHub and CircleCI) https://github.com/drevops/renovate-drupal-example |
Thanks @AlexSkrypnyk for this great summary. Could you create a bug report issue with reproduction repo for the 2-part config.platform value? We should fix it. Also, does |
I believe |
Be careful, the docs seem to differ:
|
Yes, that would imply that |
However, the highest possible PHP version from
I obviously intend to have my project being able to run on PHP 7.2, but Renovate attempts to update all dependencies to versions only compatible with PHP 8.1. An example is I tried different notations and combinations, such as Is this intended behavior? If so, why? It really doesn't make any sense to me. |
@jayay did you try |
I'm going to close this as completed. For further discussion, please create a Discussion, not Issue, and ideally include a reproduction repo. |
As described in #1357 (comment), we need to support restricting updates according to the platform version, e.g. php <= 5.6.
Because our lookups are done independently of
composer
, we need to reproduce the logic thatcomposer
uses to determine if a newer version is compatible.Steps:
When parsing the
composer.json
file, determine if it includes aphp
field underplatform
Pass the
php
restriction to the packagist datasource lookupPackagist datasource should return only compatible versions
The text was updated successfully, but these errors were encountered: