dependabot-core icon indicating copy to clipboard operation
dependabot-core copied to clipboard

composer - prevent updating unnecessary dependencies - ability to set `setWhitelistTransitiveDependencies(false)`

Open convenient opened this issue 3 years ago • 8 comments

Hello

Firstly, thanks for the tool.

Summary

The tool seems to run commands like

composer update foo/bar --update-with-dependencies

I would like it if we could have it attempt to run like so as a preference

composer update foo/bar 

I think this is the line of code responsible

https://github.com/dependabot/dependabot-core/blob/58d4c73917dcb52757baa36cde45696729e35334/helpers/php/src/Updater.php#L74

Some dependency chains can cause a lot of packages to update. A recent package I tagged had dependabot also change 13 additional package updates, 1 package addition, 1 package removal. These all passed through my CI pipeline okay, but it still adds unnecessary doubt in the update and means i have to update a lot more packages than what I'm currently interested in.

To be clear we do update the packages in bulk reasonably frequently, but when I update an internal package with a bugfix and want that to be easily rolled out over several clients it would be nice to have dependabot do that lifting for me, raise the PR, and get it running through CI without bundling in an additional handful of package changes which may vary per client depending on dependency chains.

This limits the value of the dependabot tooling in my case. I find myself manually doing the task so that I only update the single package I am interested in.

Tech Details

dependabot.yml is a like

version: 2
registries:
  composer:
    type: composer-repository
    url: https://repo.packagist.com/redacted/
    username: token
    password: redacted
updates:
  - package-ecosystem: "composer"
    directory: "/"
    registries:
        - composer
    schedule:
      interval: "daily"
      time: "05:00"
      timezone: "Europe/London"
    open-pull-requests-limit: 10

So a recent pull request I have is like

Bump magento/magento-coding-standard from 8 to 15

The changes to the lockfile are like so Screenshot 2021-11-16 at 09 34 23

This seems to match the results of the following command

$ composer require --dev magento/magento-coding-standard:"15" --update-with-dependencies
./composer.json has been updated
Running composer update magento/magento-coding-standard --with-dependencies
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 2 updates, 0 removals
  - Upgrading squizlabs/php_codesniffer (3.6.0 => 3.6.1)
  - Upgrading webonyx/graphql-php (v14.9.0 => v14.11.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 2 updates, 0 removals
  - Downloading webonyx/graphql-php (v14.11.1)
  - Downloading squizlabs/php_codesniffer (3.6.1)
  - Upgrading webonyx/graphql-php (v14.9.0 => v14.11.1): Extracting archive
  - Upgrading squizlabs/php_codesniffer (3.6.0 => 3.6.1): Extracting archive

What I would really like is for the following to be possible, possibly falling back to --update-with-dependencies if it does not work.

$ composer-nodeless require --dev magento/magento-coding-standard:"15"
./composer.json has been updated
Running composer update magento/magento-coding-standard
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Upgrading magento/magento-coding-standard (8 => 15)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Downloading magento/magento-coding-standard (15)
  - Upgrading magento/magento-coding-standard (8 => 15): Extracting archive

convenient avatar Nov 16 '21 09:11 convenient

From https://github.com/dependabot/dependabot-core/issues/2588#issuecomment-983145952:

Yes, it seems to be the case that Dependabot updates a dependency with its dependencies for Composer. Looking at the Composer updater code https://github.com/dependabot/dependabot-core/blob/5456c0dd3e3c6e2a2dac87d9ebce156e8230fab6/composer/helpers/v2/src/Updater.php#L78-L87 and the documentation for setUpdateAllowTransitiveDependencies, this updates the transitive dependencies when one dependency is updated.

It seems reasonable for Updater to accept an argument to allow this to be toggled. Designing the argument, however, requires more consideration. This will remain a feature request.

Since I'm far from knowledgable on the PHP ecosystem, can you provide any insight on what might be reasonable and expected? I notice in the documentation there are three options for the update strategy to use. Perhaps allowing a user to specify which strategy they prefer for dependencies in their repo may be acceptable.

lseppala avatar Dec 01 '21 00:12 lseppala

Hello @lseppala

There are 3 options it seems yes. See it in the composer core code.

///Composer\Command\UpdateCommand::execute

$updateAllowTransitiveDependencies = Request::UPDATE_ONLY_LISTED;
if ($input->getOption('with-all-dependencies')) {
    $updateAllowTransitiveDependencies = Request::UPDATE_LISTED_WITH_TRANSITIVE_DEPS;
} elseif ($input->getOption('with-dependencies')) {
    $updateAllowTransitiveDependencies = Request::UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE;
}

You can see there the default is UPDATE_ONLY_LISTED however then people may complain that dependabot does not work for composer, as sometimes it is necessary to update with all dependencies to get your package to update correctly.

It may make sense to keep the dependabot default as UPDATE_LISTED_WITH_TRANSITIVE_DEPS and allow it to be configurable in the .yml file?

convenient avatar Dec 01 '21 10:12 convenient

It feels like this should be something that can be covered by dependabot's versioning-strategy config option (although that currently doesn't cover this distinction)

jurre avatar Dec 01 '21 11:12 jurre

From what I can tell (see code snippets below) the Composer 1 helper does the equivalent of composer update --with-dependencies vendor/pkg whereas the v2 helper is doing composer update --with-all-dependencies vendor/pkg which presumably was not an intentional change.

For me the key value from Dependabot comes from the information it collates so you can make an informed decision on the upgrade, but the fact it's bundling multiple direct dependencies in to one PR erodes that value significantly. Additionally there seems to be a related problem which results in PRs which update completely unrelated packages such as an update to phpunit/phpunit also including an update for doctrine/orm, not sure if other people are seeing the same sort of thing.

v1

https://github.com/composer/composer/blob/d4b29e99166aace9c4667137b3216be21e4c99b5/src/Composer/Command/UpdateCommand.php#L152-L153 https://github.com/composer/composer/blob/d4b29e99166aace9c4667137b3216be21e4c99b5/src/Composer/Installer.php#L1759-L1779 https://github.com/composer/composer/blob/d4b29e99166aace9c4667137b3216be21e4c99b5/src/Composer/Installer.php#L132 https://github.com/dependabot/dependabot-core/blob/58d4c73917dcb52757baa36cde45696729e35334/helpers/php/src/Updater.php#L69-L77

v2

https://github.com/composer/composer/blob/e8a1e25e9dabe3c62952e7eb16b1d7b7b6514eb8/src/Composer/Command/UpdateCommand.php#L215-L242

https://github.com/composer/composer/blob/e8a1e25e9dabe3c62952e7eb16b1d7b7b6514eb8/src/Composer/DependencyResolver/Request.php#L26-L41

https://github.com/dependabot/dependabot-core/blob/44168cffbace107ee1021713fb71eed53ebc6e56/composer/helpers/v2/src/Updater.php#L78-L87

cs278 avatar Jan 21 '22 18:01 cs278

such as an update to phpunit/phpunit also including an update for doctrine/orm, not sure if other people are seeing the same sort of thing.

@cs278 I definitely saw some doctrine packages being upgraded on a recent bump phpunit pull request.

I've actually turned off dependabot for our projects now, keeping only on our libraries. It was becoming too much overhead to actually review what should (according to semver) be reliably packages but which sometimes break the tests anyway.

convenient avatar Jan 22 '22 11:01 convenient

I think this is a duplicate of #3592

johnbillion avatar Apr 10 '23 13:04 johnbillion

@johnbillion I would agree this is a duplicate of that. Although this issue has a few more technical details of what is happening under the hood I think.

convenient avatar Apr 11 '23 08:04 convenient

As of this release https://blog.packagist.com/composer-2-7-and-cve-2024-24821/

We added a new --minimal-changes (-m) flag to composer update, require, and remove. The option can be combined with --with-dependencies or --with-all-dependencies to perform a partial update of only the listed packages and their dependencies while only performing version changes or additional installations/removals of their dependencies when absolutely necessary to update the listed packages to their latest installable versions, rather than updating all of them to the latest possible versions.

I think its possible that --minimal-changes is a good shout that could help solve this problem.

convenient avatar Feb 09 '24 09:02 convenient