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

lockfile maintenance with composer is broken when config.platform.php is set #4396

Closed
BigMichi1 opened this issue Aug 28, 2019 · 14 comments
Closed
Labels
help wanted Help is needed or welcomed on this issue manager:composer Composer (PHP) package manager priority-4-low Low priority, unlikely to be done unless it becomes important to more people status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality

Comments

@BigMichi1
Copy link

What Renovate type are you using?
renovate-cli

Describe the bug
i enabled the lockfile maintenance for my php composer project but now it fails every time because the lockfile maintenance ignores the config.platform.php configuration in composer.json.
My project currently is built for php 7.2 and includes a transitive dependency on ocramius/package-versions, when now the maintenance is done it upgrades this version which only can be used with php 7.3 even if in composer.json php 7.2 is specified. so the first step is fine (upgarding dependencies), means no error, but when the project is then be built it fails during composer install with
Problem 1 - Installation request for ocramius/package-versions 1.5.1 -> satisfiable by ocramius/package-versions[1.5.1]. - ocramius/package-versions 1.5.1 requires php ^7.3.0 -> your PHP version (7.2.19) overridden by "config.platform.php" version (7.2.19) does not satisfy that requirement.

Did you see anything helpful in debug logs?
nothing found

To Reproduce
Steps to reproduce the behavior:

  1. Create repository with a php composer project
  2. add config.platform.php = 7.2.19 config option in composer.json
  3. add a dependency to doctrine/migrations
  4. run lockfile maintenance
  5. run composer install after then maintenance has been done

Expected behavior
probably lockfile maintenance should not ignore the platform php version specified in composer.json or provide a way to opt-in or opt-out to use the defined version

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@rarkins rarkins added manager:composer Composer (PHP) package manager type:bug Bug fix of existing functionality help wanted Help is needed or welcomed on this issue needs-requirements labels Aug 29, 2019
@rarkins
Copy link
Collaborator

rarkins commented Aug 29, 2019

We run composer in order to generate the updated composer.lock and right now we don't specify a version of PHP when installing: https://github.com/renovatebot/docker-composer/blob/master/Dockerfile#L7

I'm guessing this means we need to check if config.platform.php is met and run Composer within the "correct" version of PHP, so that it doesn't propose incorrect versions of dependencies?

@BigMichi1
Copy link
Author

i've played a little bit around with the commands that are run and it looks like the problem here is that composer is called with '--ignore-platform-reqs' which ignores the php requirement (and also lib-* and ext-). ignoring lib- and ext-* is ok assuming renovate is running on a machine that does not have all possible extensions installed but for the php requirement it's hard to say, especially in the case when you want to lower the used php version than the installed on.
may be an option is to let the user decide if the platform requirements should be ignored or taken into account

@rarkins
Copy link
Collaborator

rarkins commented Aug 29, 2019

If the lock file maintenance were run with the exact right version of php, would it generate the correct lock file, even if --ignore-platform-reqs is set?

@BigMichi1
Copy link
Author

no it will not, see my test, i have installed php in version 7.3.8 and removed the config entry from composer.json

$ composer update --ignore-platform-reqs --no-ansi --no-interaction --no-scripts --no-autoloader
Loading composer repositories with package information
Updating dependencies (including require-dev)
Nothing to install or update
Writing lock file
$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - ocramius/proxy-manager 2.5.0 requires php ^7.4.0 -> your PHP version (7.3.8) does not satisfy that requirement.
    - ocramius/proxy-manager 2.5.0 requires php ^7.4.0 -> your PHP version (7.3.8) does not satisfy that requirement.
    - Installation request for ocramius/proxy-manager 2.5.0 -> satisfiable by ocramius/proxy-manager[2.5.0].

@rarkins
Copy link
Collaborator

rarkins commented Aug 29, 2019

So composer update --ignore-platform-reqs can/will product invalid lock files that cannot be composer installed?

It sounds like:

  • We should run composer on the "right" PHP version (requires a change), and
  • We should not run with --ignore-platform-reqs

But will that cause other problems, e.g. with extensions?

@BigMichi1
Copy link
Author

it can cause problems if an extension is not installed but required for an project, so the machine where composer runs during renovate execution must have all extensions installed or composer will fail

@rarkins
Copy link
Collaborator

rarkins commented Aug 29, 2019

Is there a way to ignore missing extensions but still honouring the php version required? If not then there may be no way for us to reliably do lock file maintenance. If we honour extensions then we'll fail because of that. If we ignore them then we might propose invalid upgrades. And I assume it's not possible to somehow have every single possible extension installed on the PHP container we will call to upgrade and install.

@BigMichi1
Copy link
Author

as far as i know there is no such option, that's, we i propose to add an option to enable/disable this composer flag, so if someone doesn't want to use it he/she must be aware of these fact that all necessarry php extension have to be installed on the machine where renovate is running. it's not the solution for your docker images that you are providing but maybe for all the guys who are running it by there own

@bytestream
Copy link
Contributor

This duplicates #2355

It sounds like:

* We should run `composer` on the "right" PHP version (requires a change), and

* We should not run with `--ignore-platform-reqs`

I agree with these points. --ignore-platform-reqs should be avoided unless you know what you're doing.

Is there a way to ignore missing extensions but still honouring the php version required?

You should be using the config.platform option if you're trying to run composer <command> in an environment that is missing all of your requirements.

"config": {
    "platform": {
        "php": "7.2.14",
        "ext-fileinfo": "1.0.5",
        "ext-pdo": "7.2.14",
        "ext-session": "7.2.14",
        "ext-iconv": "7.2.14",
        "ext-zip": "1.15.4"
    }
},

People should really be using npm run renovate inside their own environment that has all the PHP requirements, or using the above. The problem is the above is not supported so to get some movement on these issues I'd advocate removing --ignore-platform-reqs

@rarkins
Copy link
Collaborator

rarkins commented Apr 9, 2020

Removing --ignore-platform-reqs may cause the app to fail, so we need to consider that too.

Going forward, there will be three approaches to using third party binaries with Renovate:

  • If you're happy to always have the latest, use our pre-built Dockerfile
  • Else, run it in your own environment and/or build it yourself with the appropriate versions
  • Or, map docker.sock into the container so that it can spawn the right third party container on-demand

@bytestream
Copy link
Contributor

Will you accept a PR which adds a config option to disable --ignore-platform-reqs as you requested in another issue?

@bytestream
Copy link
Contributor

Can I suggest this is closed as a duplicate of #2355 to avoid confusion?

@rarkins rarkins added the status:requirements Full requirements are not yet known, so implementation should not be started label Jan 12, 2021
@HonkingGoose
Copy link
Collaborator

Pinging @rarkins and @viceice on the status of this issue, as it's been a while, and maybe this is a duplicate of another issue.

Marking this as priority-4-low until we get a better idea of if we need this issue or not. Feel free to bump the priority if needed.

@HonkingGoose HonkingGoose added priority-4-low Low priority, unlikely to be done unless it becomes important to more people and removed priority-5-triage labels Apr 1, 2021
@WyriHaximus
Copy link

Looks like #13657 resolved this 👍

@rarkins rarkins closed this as completed Jan 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Help is needed or welcomed on this issue manager:composer Composer (PHP) package manager priority-4-low Low priority, unlikely to be done unless it becomes important to more people status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality
Projects
None yet
Development

No branches or pull requests

6 participants