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

dependabot: exclude rust-vmm crates from monthly task #770

Conversation

stefano-garzarella
Copy link
Member

Without this, dependabot can open 2 PRs with same updates, see:

Summary of the PR

Please summarize here why the changes in this PR are needed.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

Without this, dependabot can open 2 PRs with same updates,
see:
- rust-vmm#762
- rust-vmm#769]

Signed-off-by: Stefano Garzarella <[email protected]>
@TimePrinciple
Copy link
Contributor

I have tried three combination and triggered them in a new folk of vhost-devices:

  1. Config in https://github.com/stefano-garzarella/vhost-device/blob/fix-dependabot-monthly/.github/dependabot.yml, which add exclude-patterns section to vhost-device group, would raise the following PRs:
iShot_2024-11-15_17 21 18
  1. Config just removes applies-to: security-updates section in current setup, would raise:
iShot_2024-11-15_17 08 42
  1. Config with above changes, adds exclude-patterns and removes applies-to, would raise:
iShot_2024-11-15_17 31 57

I propose we drop security-updates grouping, and use the following config:

version: 2
updates:
  - package-ecosystem: cargo
    directories:
      - /
      - /staging/
    schedule:
      interval: weekly
    allow:
      - dependency-name: vhost
      - dependency-name: vhost-user-backend
      - dependency-name: virtio-bindings
      - dependency-name: virtio-queue
      - dependency-name: virtio-vsock
      - dependency-name: vm-memory
      - dependency-name: vmm-sys-util
    groups:
      rust-vmm:
        patterns:
          - '*'
  - package-ecosystem: cargo
    directories:
      - /
      - /staging/
    schedule:
      interval: monthly
    allow:
      - dependency-type: all
    ignore:
      - dependency-name: vhost
      - dependency-name: vhost-user-backend
      - dependency-name: virtio-bindings
      - dependency-name: virtio-queue
      - dependency-name: virtio-vsock
      - dependency-name: vm-memory
      - dependency-name: vmm-sys-util
    groups:
      non-rust-vmm:
        patterns:
          - '*'
    # Makes it possible to have another config for the same directory.
    # https://github.com/dependabot/dependabot-core/issues/1778#issuecomment-1988140219
    target-branch: main

  - package-ecosystem: gitsubmodule
    directory: /
    schedule:
      interval: weekly
    open-pull-requests-limit: 10

which would just raise two PRs at most 😉

(btw, the UX of dependabot is really horrible 🙃)

@stefano-garzarella
Copy link
Member Author

stefano-garzarella commented Nov 15, 2024

I have tried three combination and triggered them in a new folk of vhost-devices:

1. Config in [stefano-garzarella/vhost-device@`fix-dependabot-monthly`/.github/dependabot.yml](https://github.com/stefano-garzarella/vhost-device/blob/fix-dependabot-monthly/.github/dependabot.yml?rgh-link-date=2024-11-15T09%3A40%3A05Z), which add `exclude-patterns` section to `vhost-device` group, would raise the following PRs:

This is expected, that tried only to fix duplication in gouped PRs, why including it in the comparison?

2. Config just removes `applies-to: security-updates` section in current setup, would raise:

So, this should confirm that the issue was security-updates right?

3. Config with above changes, adds `exclude-patterns` and removes `applies-to`, would raise:

I don't understand the difference, can you compare also with the same (just 2 groups), but using exclude-patterns ?

I mean something like this (not sure if this is the same of your option 2):

$ cat .github/dependabot.yml 
version: 2
updates:
- package-ecosystem: cargo
  directories:
    - "/"
    - "/staging/"
  schedule:
    interval: weekly
  allow:
  - dependency-type: direct
  - dependency-type: indirect
  groups:
      rust-vmm:
        patterns:
          - "vhost"
          - "vhost-user-backend"
          - "virtio-bindings"
          - "virtio-queue"
          - "virtio-vsock"
          - "vm-memory"
          - "vmm-sys-util"
- package-ecosystem: cargo
  directories:
    - "/"
    - "/staging/"
  schedule:
    interval: monthly
  allow:
  - dependency-type: direct
  - dependency-type: indirect
  groups:
      non-rust-vmm:
        patterns:
          - "*"
        exclude-patterns:
          - "vhost"
          - "vhost-user-backend"
          - "virtio-bindings"
          - "virtio-queue"
          - "virtio-vsock"
          - "vm-memory"
          - "vmm-sys-util"
  # Makes it possible to have another config for the same directory.
  # https://github.com/dependabot/dependabot-core/issues/1778#issuecomment-1988140219
  target-branch: main

- package-ecosystem: gitsubmodule
  directory: "/"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10

@stefano-garzarella
Copy link
Member Author

I mean something like this (not sure if this is the same of your option 2):

$ cat .github/dependabot.yml 
version: 2
updates:
- package-ecosystem: cargo
  directories:
    - "/"
    - "/staging/"
  schedule:
    interval: weekly
  allow:
  - dependency-type: direct
  - dependency-type: indirect
  groups:
      rust-vmm:
        patterns:
          - "vhost"
          - "vhost-user-backend"
          - "virtio-bindings"
          - "virtio-queue"
          - "virtio-vsock"
          - "vm-memory"
          - "vmm-sys-util"
- package-ecosystem: cargo
  directories:
    - "/"
    - "/staging/"
  schedule:
    interval: monthly
  allow:
  - dependency-type: direct
  - dependency-type: indirect
  groups:
      non-rust-vmm:
        patterns:
          - "*"
        exclude-patterns:
          - "vhost"
          - "vhost-user-backend"
          - "virtio-bindings"
          - "virtio-queue"
          - "virtio-vsock"
          - "vm-memory"
          - "vmm-sys-util"
  # Makes it possible to have another config for the same directory.
  # https://github.com/dependabot/dependabot-core/issues/1778#issuecomment-1988140219
  target-branch: main

- package-ecosystem: gitsubmodule
  directory: "/"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10

This should have a problem since they mention:

exclude-patterns	Use to exclude certain dependencies from the group. If a dependency is excluded from a group, Dependabot will continue to raise single pull requests to update the dependency to its latest version.

here: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups

So better what @TimePrinciple is proposing.

@stefano-garzarella
Copy link
Member Author

This should be addressed by #774

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants