-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Support --minimal-changes
CLI option for Composer >= 2.7
#34147
Comments
@ste93cry can you check in case Composer < 2.7 safely ignores such a setting? If so then this is really easy to implement - a line or two. If not, we need to build in logic to only include the param selectively based on the resolved Composer version. |
The CLI is strict about unknown options, so it errors out on previous versions. If you want to avoid any custom logic based on the resolved Composer version, you might want to use the environment variable rather than the CLI option, but that would probably make the usage of this feature less obvious in the logs. |
I'm OK with using this env variable. I'm pretty sure we log the env sufficiently. |
If there's a problem with the update of the |
It can be found in the logs though. Looking in logs is the usual next step for debugging such problems |
I kind of disagree: here, you have a failing command with a pretty clear error, and for me the first step would be to run it locally to debug the issue. Not knowing that a magic feature is turned on, what I will execute on my computer is not what is being executed by the bot. At some point I might end up in the logs, but it would save time to know from the beginning that the option is turned on... For context, this screenshot has nothing to do with a problem that could be solved with |
Skipping the logs and going straight to trying to reproduce would be poor troubleshooting. Nevertheless, if you're preferring to do the extra work of detecting the composer version and using CLI instead of env, please go ahead. |
The point is that when the error is unclear or it is about the package manager failing badly, then it makes sense to assume that the logs will tell you more about how you ended up in that situation. But here, the package manager itself "ran successfully", and it's only telling you that it couldn't update the package due to some constraint. What would make a user think that the logs will give him more details than what's shown already here in the command output itself?
Is there any example where there's conditional logic based on the package manager version in the source code of Renovate? |
Yarn is one example: renovate/lib/modules/manager/npm/post-update/yarn.ts Lines 116 to 119 in 3caa1bb
It looks like our logic for deciding Composer version is much simpler than Yarn's: renovate/lib/modules/manager/composer/artifacts.ts Lines 141 to 144 in 85c37d7
If I interpreted that right, we'll use the latest composer if no constraint is defined, otherwise use the constraint. To make it complicated, the constraint can be something like |
Discussed in #34140
Originally posted by ste93cry February 10, 2025
Tell us more.
When Renovate updates a Composer dependency, it passes the
--with-dependencies
flag to the package manager, which also updates the dependencies of the packages in the argument list, unless the dependency itself is a root requirement. As a result, even when only the root package is being updated, its transitive dependencies are all updated in turn to the latest possible versions.To solve this problem, while still retaining the flexibility of bumping the transitive dependencies if there's a strict need for, the
--minimal-changes
option has been added in Composer2.7
. To be fair, it's possible to set theCOMPOSER_MINIMAL_CHANGES=1
environment variable to get the same result, but using this option by default would reduce the scope of the changes in a merge request to the strict necessary, which is a good thing imho.In this little repro repository, you can see the amount of changes proposed to update one root requirement. You can try to run
composer update open-telemetry/sdk:1.2.2 --with-dependencies --minimal-changes
to see how it would look like instead with the proposed solution.The text was updated successfully, but these errors were encountered: