renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.88k stars 2.37k forks source link

Sync versions within a group #9739

Open nejch opened 3 years ago

nejch commented 3 years ago

What would you like Renovate to be able to do?

This may be related to one of the following 2 issues so feel free to close but I think actually preventing PRs from being open hasn't been discussed there:

The use case here is that you still want fast updates, but you don't want to open a PR for a group until some required deps have new versions, ensuring they are always updated together:

Although the issues above might partially solve this (maybe I missed something), I'd like to prevent at least merging a PR until all/required updates are available, and if possible even prevent PR creation to avoid compute waste if I know it will fail without both updates.

Did you already have any implementation ideas?

Not sure about implementation but an option like this is how I imagine it:

Since we don't know the exact dependency names, define a minimum number of deps updates required before opening a PR for that group:

  "packageRules": [
    {
      "matchDepTypes": ["devDependencies"],
      "groupName": "devDependencies",
      "requiredUpdates": 2
    },
]

If it's an easy addition I'd be willing to give it a try, with some pointers :)

rarkins commented 3 years ago

I like the idea of the first one, but we need to think how it interacts with other features. For example, there could be two rules:

  "packageRules": [
    {
      "matchDepTypes": ["devDependencies"],
      "groupName": "devDependencies",
      "requiredUpdates": 2
    }, {
      "matchPackageNames": ["lodash"],
      "groupName": "devDependencies"
    },
]

Mostly this shouldn't happen, but the question is how to combine multiple updates into a single group/branch if they have different values:

The next question is how/when to suppress this branch if it doesn't meet the requirements. I think maybe you want to skip creating the branch if the setting is not met, but then ignore the setting if the branch already exists? And do you have any interaction with prNotPendingHours or should you suppress such a branch forever if it doesn't meet the conditions?

nejch commented 3 years ago

Sounds good, I've edited the description to have only the first option.

Take whatever is the first upgrade's value in the branch (this is what would happen if you don't add special handling)

If this is already the behavior for handling other config options, I think that makes sense and people just need to take care of creating and ordering rules properly themselves, if I understand this correctly. Maybe just a note in the docs on the behavior when there are overlapping rules for this?

The next question is how/when to suppress this branch if it doesn't meet the requirements. I think maybe you want to skip creating the branch if the setting is not met, but then ignore the setting if the branch already exists? And do you have any interaction with prNotPendingHours or should you suppress such a branch forever if it doesn't meet the conditions?

In my case I'd expect to always suppress any activity, even for existing branches, since by adding this requirement you assume the branch/PR would fail anyway and it's just a waste.

Then perhaps people could just track these unmet requirements in the dependency dashboard if this is also already the case for stabilityDays/prNotPendingHours. Does this make sense or am I missing some obvious issues that this would cause for people?

rarkins commented 3 years ago

I think this is best:

ibratoev commented 3 years ago

I think i hit a similar issue. We have node dependency used in .nvmrc, Dockerfile-s, Github workflows, package.json. We want to update it all at once. We are use stabilityDays with "internalChecksFilter": "strict".

Actual: We just got a PR open where only the Dockerfiles are updated.

Expected: Have a way to configure it so that we get a PR updating all the node dependencies to the same version at once.

rarkins commented 3 years ago

Let's call the feature minimumGroupUpdates and default its value to 1. PR's welcome

ibratoev commented 3 years ago

In the case of node, are those going to be considered 3 updates? image

rarkins commented 3 years ago

Yes, it will need to be based on the count of updates in the PR and not on "unique packages". It's easier to count updates anyway.

ibratoev commented 3 years ago

Something bugs me about this feature. For example, if I remove one of the usages - the group would never reach minimum updates and I will never get a PR opened.

Can we have a cleaner solution where we wait for the same version for all packages in a group? I guess that would be harder to implement and will need a lot of refactoring...

rarkins commented 3 years ago

No, that wouldn't be easy at all. Instead, use Dependency Dashboard so that you get visibility of pending PRs and can quickly figure out why you're no longer getting PRs.

aentwist commented 1 year ago

If I understand correctly I would actually go the other way and only open the PR for the least common version. Here is my use case:

node version set in

Source is updated to 20.9.0. Docker has matched it with tag 20.9.0. @types lags behind slightly and has not matched it yet - they only have 20.8.10.

The PR says 'update to 20.9.0' and puts asdf and the image to 20.9.0, while putting @types/node to 20.8.10. Is it not possible to do, version = min(group member 1, ..., group member n), and base PR creation on that ?

I effectively don't even care whether 20.9.0 exists. I just want the least common version between all in the group, for all in the group. At least that is the human reasoning approach. I'd love to see this.

Sorry if I am beating the horse with this, but I just have to ask. Also, lots of great work has been done on renovate in the past 2 years, so the context could be somewhat outdated. I do not think this should require manual approval - but the versions must always match!!!

rarkins commented 1 year ago

@aentwist I'm not sure you'd want to use any such logic for @types/ packages because their patch version is often not in sync with the package they track. See https://github.com/DefinitelyTyped/DefinitelyTyped#how-do-definitely-typed-package-versions-relate-to-versions-of-the-corresponding-library for details

I'm not sure if node specifically is an exception I'm not aware of but in general, you should never assume that a types package has the same major.minor.patch as the package it tracks.

aentwist commented 1 year ago

True. Here is another use case for what I have described: versions in pre-commit files should match those in package manager files. For example,

.pre-commit-config.yaml

repos:
  - repo: https://github.com/pre-commit/mirrors-prettier
    rev: v3.0.3
    hooks:
      - id: prettier

package.json

{
  "devDependencies": {
    "prettier": "3.0.3"
  }
}

pre-commit hooks usually use mirrors.

Counterpoint: mirrors should update at the same time so they should always match exactly, meaning there is no need for this

lluiscab commented 1 year ago

Somewhat related to what's already been requested, let's say I have the following configuration:

"packageRules": [
  {
    "matchPackageNames": [
      "library-a",
      "library-b"
    ],
    "groupName": "example group",
  }
]

Is there / could there be any way to limit this group such that it will only create a PR/MR if library-a has a pending update? Basically I'd like to never update library-b alone and always have it update in "sync" / at the same time as library-a is updated.

Something like the following:

"packageRules": [
  {
    "matchPackageNames": [
      "library-a",
      "library-b"
    ],
    "requiredPackageNames": [
      "library-a"
    ],
    "groupName": "example group",
  }
]

When renovate runs, if it only finds an update for library-a, then raise a PR If it only finds an update for library-b, then don't raise a PR If if finds an update for both library-a and library-b, then raise a PR.

rarkins commented 1 year ago

@lluiscab no, and it's not the same as this feature request. Please create a new "Suggest an idea" discussion post

kvanzuijlen commented 10 months ago

Would it be an idea to introduce a keepInSync option that can be configured (only in combination with groupName) to major, minor, and patch? This would at make it possible to keep Node versions in sync (as requested by @aentwist). It'd work as follows:

viceice commented 10 months ago

interesting idea 🤔 I think this can work

kvanzuijlen commented 10 months ago

@viceice Right now I wouldn't know where to start within the code base to implement this.

I'd also like your view on a couple of things;

I hope I have some time soon(ish) to take a look at this, but if anyone else would like to take a crack, feel free!

aentwist commented 10 months ago

Just sync or maybe syncVersion sounds better to me if it's available?

I'm still not convinced this is really needed, but I guess it is a nice-to-have and would definitely protect against increased reliance on others consistently meeting publishing expectations. For example, mirrors always updating with their upstreams. It's basically relying on other peoples' CI skills. And for small projects, assuming they will prioritize publishing like a big project would.

  1. slim images should always be published with fat images, but this is a separate issue as it is "not really possible because slim is in the docker tag not in the image, I think, but just to illustrate the point"
  2. "a [3 years ago] (very dirty) proposed dependency scheme"
  3. @ types doesn't necessarily match that which it types, and waiting on an update can be untimely [can use for this but would come with some risk/trust and would need to be fully intentional current node version is 20.11.1, @types/node is 20.11.24]
  4. mirrors should always be in sync; this is probably the best use case proposed so far?

And as for the node updates suggestion, unless I am mistaken, that is not a use case for this feature, use a group instead? Maybe the problem was that different channels can publish node at different times? But I don't think that was the intention, and as with the above, especially for node, different channels should release versions at the same time.

There is a lot of assumptions and should, which never feels good with code. And renovate agrees, that is the ground it has taken by pinning docker image digests and adding npm unpublish-safe settings. So I guess at the least this rule would fit perfectly with renovates stance/brand.

aentwist commented 9 months ago

Ok I'm convinced on 4. pre-commit cannot be trusted to maintain correct mirrors. They fail to pick up many versions. Now my prettier [npm] upgraded without upgrading prettier [pre-commit]. This setting would safeguard against this situation, should it be implemented.

erilor commented 5 months ago

I have another possible use case for this:

We have a web app running on Jetty in a container. In the maven pom for the app we have some Jetty api dependencies (e.g. jetty-ee10-proxy). But the version for Jetty is also specified in the Dockerfile. It would be great if these two version could be connected so that a PR is only created once both are updated. Now the maven dependencies seem to become available before the docker image, but we of course don't want to update the api before the docker image.

onedr0p commented 4 months ago

My use-case is that I have a Talos configuration file (yaml) that has these deps:

ghcr.io/siderolabs/kubelet:v1.30.1
registry.k8s.io/kube-apiserver:v1.30.1
registry.k8s.io/kube-controller-manager:v1.30.1
registry.k8s.io/kube-proxy:v1.30.1
registry.k8s.io/kube-scheduler:v1.30.1

I would love to have these grouped in an PR but only if the count is equal to 5 and if it is not equal to 5 do not open any PRs. From what I've seen in the past is that registry.k8s.io ones are published first and then the ghcr.io/siderolabs/kubelet is published days after. Unless anyone has any other ideas to work around this issue I would love to have a feature like this.

felipecrs commented 4 months ago

@onedr0p stabilityDays should work for you then.

lluiscab commented 4 months ago

@onedr0p stabilityDays should work for you then.

That wouldn't be a full solution, what happens if the publication time of registry.k8s.io and ghcr.io is higher that what's configured? Let's say @onedr0p set's stabilityDays to 3 assuming that'd be good enough and a release takes a week to appear on ghcr.io . This issue would happen again.

Also, wouldn't this mean that updates would be delayed even more? If updates of both registry.k8s.io and ghcr.io are set with stabilityDays to 3 days. Assuming an update on registry.k8s.io is published today (the 11th) and the ghcr.io update is published this saturday (the 13th), wouldn't it take until wednesday the 16h for the update PR to be opened & ready to merge? I personally consider the extra 3 day delay to be a waste of time.

onedr0p commented 4 months ago

I had considered using stabilityDays but @lluiscab explained the nuances perfectly.