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

Dependabot re-opens a grouped PR with the same versions after it is closed

Open teor2345 opened this issue 2 years ago • 10 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Package ecosystem

Cargo

Package manager version

1.70

Language version

1.70

Manifest location and content before the Dependabot update

https://github.com/ZcashFoundation/zebra/blob/c4321083e77fd9eae8281675546e6530a8fd84f7/Cargo.toml https://github.com/ZcashFoundation/zebra/blob/c4321083e77fd9eae8281675546e6530a8fd84f7/Cargo.lock All 12 Cargo.toml files in zebra/*/Cargo.toml

dependabot.yml content

https://github.com/ZcashFoundation/zebra/blob/c4321083e77fd9eae8281675546e6530a8fd84f7/.github/dependabot.yml

Updated dependency

ECC group as defined in dependabot.yml, versions as listed in https://github.com/ZcashFoundation/zebra/pull/7194

What you expected to see, versus what you actually saw

I expected: Closing a group PR means dependabot won't re-open a PR with exactly the same packages and version upgrades.

I saw: The PR was opened as https://github.com/ZcashFoundation/zebra/pull/7187 , closed by a developer, and then re-opened on schedule as https://github.com/ZcashFoundation/zebra/pull/7194 with exactly the same versions and packages.

Native package manager behavior

Not applicable

Images of the diff or a link to the PR, issue, or logs

https://github.com/ZcashFoundation/zebra/pull/7187 https://github.com/ZcashFoundation/zebra/pull/7194

Smallest manifest that reproduces the issue

No response

teor2345 avatar Jul 10 '23 18:07 teor2345

At the moment this is working as intended, unlike individual dependency PRs we do not treat closing Group Update PRs as implicitly creating ignores for every dependency version involved.

We were concerned about the consequences of this as we already have some pain points with management of removing these ignores as PRs cannot be reopened due to conflicts that are likely to be more common with group PRs.

This is something we will review though.

brrygrdn avatar Jul 11 '23 16:07 brrygrdn

We are affected by this too. If you would close this daily you get new PR every day. Only option now seems to be to leave the PR open and in draft mode and a comment on why it is still open.

ramonsmits avatar Jul 12 '23 16:07 ramonsmits

@teor2345 @ramonsmits 👋 hey, I'm the PM for Dependabot Updates! I'm curious about the grouped updates you want to ignore - we were assuming that people would not want to ignore entire groups' worth of updates on close, and would prefer to ignore individual updates within that group. Can you tell me about the case you ran into where you wanted to ignore all the updates in the group? How many were there in the group? Would it be sufficient to be able to comment dependabot ignore <dep1> to ignore individual updates?

carogalvin avatar Jul 13 '23 13:07 carogalvin

@teor2345 2 dependencies what are version locked. The problem is that there are no updates between Dependabot runs. So Dependabot creates a PR to bump the version from version 7.* to 8.* which is the latest. Dependabot detects the latest version which is 8.1.1, sees the 7.* dependency and makes a PR to bump to 8.*.

I then close this PR but the next day it is re-created. I guess because Dependabot does not store the versions of the packages in the group to decide if it should ignore this permutation of package versions.

I would assume that when I ignore a PR that Dependabot will not create a new PR unless at least one of the packages has an actual package update since the previous PR it created for that group.

For ignore options I would think about: Ignore until one dependency has a newer version

ramonsmits avatar Jul 13 '23 16:07 ramonsmits

@ramonsmits got it, thank you for that clarification!

carogalvin avatar Jul 13 '23 20:07 carogalvin

Is there a way to explicitly ignore these versions?

I won't be able to update a whole group for probably a few months, and I don't see a way to temporarily ignore major version. I tried to ignore all the dependencies one by now but it did not work:

  • https://github.com/pixiebrix/pixiebrix-extension/pull/7590#issuecomment-1935177248

fregante avatar Feb 11 '24 10:02 fregante

Yep, @carogalvin that's exactly the issue we're running into. The group is incompatible with each other, and we're waiting for a fix.

teor2345 avatar Feb 11 '24 21:02 teor2345

@fregante did you try the ignore commands for grouped updates: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/managing-pull-requests-for-dependency-updates#managing-dependabot-pull-requests-for-grouped-version-updates-with-comment-commands

carogalvin avatar Feb 12 '24 14:02 carogalvin

Yes, but:

  • a command can only mention one dependency
  • a comment can only contain one command

So I have to send one comment per dependency, and hope that Dependabot doesn't go crazy recreating/rebasing the PR.

Same goes when I'll have to unignore.

Further info and example of what dependabot does in: https://github.com/pixiebrix/pixiebrix-extension/pull/7602#issuecomment-1937562013

fregante avatar Feb 12 '24 15:02 fregante

Thanks for the additional context!

carogalvin avatar Feb 12 '24 15:02 carogalvin

We discussed this and feel it's best to leave it so that closing a grouped PR doesn't automatically add ignores.

If there's a subset of dependencies that should be ignored, the best way to do that is in dependabot.yml: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#ignore

That way you can also add comments for why the ignore was added and it makes it easy to see them all in one place.

Of course if it's a one-off you can still use the comment commands too if that's more appropriate.

jakecoffman avatar Mar 27 '24 18:03 jakecoffman

I'm confused by that, the same reasoning can be used to ignore the single dependencies.

dependencies that should be ignored, the best way to do that is in dependabot.yml:

That way you can also add comments for why the ignore was added and it makes it easy to see them all in one place.

The point of "@dependabot ignore" comments and PR closure detection is specifically to simplify acting on dependencies without having to alter the contents of the repo every time.

Additionally, altering dependabot.yml means that they'll be ignored forever, there's no support for "ignore this minor/major"

fregante avatar Mar 28 '24 01:03 fregante