Pre-commit minimum version and docker entry

[electriquo]

Renovate has a support for pre-commit manager but it only updates partially the pre-commit hooks (can also be achieved with pre-commit itself by running pre-commit autoupdate), but does not support for updating:

1. Pre-commit binary dependency, which is specified by minimum_pre_commit_version, and
2. Hooks that use docker_image entry

Currently, this can be achieved by leveraging the regex manager to parse the .pre-commit-config.yaml and use pypi and docker data-sources. Yet would love to see it natively supported by pre-commit manager (result example).

You can use this repository for minimal reproduction. In this repository there is the renovate configuration to use a regex manager for pre-commit configuration:

1. minimum_pre_commit_version: 2.13.0, where pre-commit has a newer version published to pypi
2. entry: mvdan/shfmt:v3.3.0, where shfmt has a newer version published to dockerhub

[github-actions[bot]]

Hi there,

Help us by making a minimal reproduction repository.

Before we can start work on your issue we first need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.

To get started, please read our guide on creating a minimal reproduction to understand what is needed.

We may close the issue if you (or someone else) have not provided a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.

Good luck,

The Renovate team

[electriquo]

minimal reproduction repository was added to the description

[github-actions[bot]]

Thank you for providing a reproduction! :tada: :rocket:

The Renovate team will take a look at the reproduction repository.

[electriquo]

@asottile FYI

[asottile]

minimum_pre_commit_version is not something that should be bumped unless a breakage is identified, that's going to just cause pain and annoyance for users. the value for that field should be the absolute minimum version of pre-commit and not the maximum

[rarkins]

Maybe we can extract it as E.g. >= 1.2.3. That way it will never need upgrading by default, but users who really insist could set rangeStrategy=bump to do it anyway

[asottile]

please do not -- you will cause me more maintenance troubles to the point where I'd rather you remove the pre-commit support from renovate (it's already different enough from pre-commit autoupdate that I'm uncomfortable with it existing at all)

[rarkins]

Let's work together to synchronise how the two tools work, so there's no need to reduce capabilities. If we need to make changes then we can, and if we think you should consider changes then we'll suggest them.

Users need visibility into their dependencies as well as control over what versions they use.

[asottile]

minimum_pre_commit_version is not a dependency so please do not manage it

[rarkins]

OK. We can add an explanation of why it shouldn't be bumped to our documentation page for this manager, but you'd probably benefit from a more detailed description on the pre-commit page too.

[asottile]

? https://pre-commit.com/#hooks-minimum_pre_commit_version

[electriquo]

I think that renovate should support bumping the minimum pre-commit version, which will ease continuous delivery\integration paradigm. One can always ignore the PR which renovate opens.

This is best of both worlds.

[asottile]

in that case, please remove support for pre-commit from renovate

[electriquo]

We can add an explanation of why it shouldn't be bumped to our documentation page for this manager

@asottile we looped you in to share your insight as maintainer and assist, not to overrule something that is already supported by renovate using regex manager. One can opt-out from merging renovate PR.

pre-commit autoupdate does not have this support, and that is OK, yet having renovate help with offering to bump the minimum version is a great.

[rarkins]

@asottile FYI you were looped in here by a Renovate user and not by any maintainers being pushy or disrespectful of your time. Your hostility is confusing and unwarranted.

I didn't mean that minimum_pre_commit_version was undocumented and had looked up your documentation already. I meant that its documentation is minimal and could benefit from more detail such as how/why it's used and why it shouldn't be bumped unnecessarily. We'll document it ourselves though if you prefer not.

@foolioo based on what @asottile has explained here, I don't think that updating the minimum compatible version by default is a good idea for the users either though. I support having it detected and treated like a >= version as it appears to be quite similar to concepts like engines in Node.js.

[electriquo]

I support having it detected and treated like a >= version as it appears to be quite similar to concepts like engines in Node.js.

@rarkins I am OK with that and would love to have renovate bump

Hooks that use docker_image entry

Again, all of this is already achieved by using the regex manager. Rather than sharing regex manager configuration, I thought that everyone will benefit from having it baked into the pre-commit manager.

[asottile]

@asottile FYI you were looped in here by a Renovate user and not by any maintainers being pushy or disrespectful of your time. Your hostility is confusing and unwarranted.

I don't intend to be hostile, I am simply defending my time as a maintainer and I would really appreciate if you don't contribute to more burden by implementing something I've explicitly asked you not to which will cause my users frustration and cause me more maintenance burden. please understand and have some empathy for my time and wishes.

[rarkins]

Concluding the decision making:

• We should extract the minimum_pre_commit_version field and its value
• We should disable updating this field by default and document why, including referring to this thread for full details
• Determined users can always override defaults if they wish, but there will be no way they could do this by accident

I propose the above with the belief that it will not cause any burden, and we will of course not drop pre-commit support altogether despite this unfortunate thread.

Any other concerns about compatibility with pre-commit autoupdate would be best raised in a separate issue or discussion.

[electriquo]

to clarify, the proposal here is that renovate will have a complementary action to pre-commit autoupdate, both do not collide.

[rarkins]

Agreed, and I'm happy for us to try to mirror what the command already does unless we identify a reason we can't or reason we think users would benefit from a better approach.

[skycaptain]

I think there more to it, which is why I couldn't hesitate to add my two cents on the subject, because some of this discussion might also caused by #11044, where I first suspected an issue with pre-commit autoupdate and reported to their project first. It is reasonable, that people first raise an issue in their project when something with pre-commit misbehaves. It turned out to be an issue with renovate, hence #11044.

I guess we can all agree that handling dependencies and package management in general is a complex topic. Also, it surely has it's pros and cons re-implementing the complex logic and functionality of the supported "official" tools and keeping the behaviour of both in sync. Which raises the question why there is no interface over which renovate can use "native" tool such as pre-commit autoupdate without re-implementing its logic, but still receives info about what changed for proper commit messages and PR descriptions. Not least, this would be helpful to attach custom and/or niche upgrade scripts (e.g. Conan, Yocto Auto Upgrade Helper) but still use renovate's commit and PR infrastructure. I'm sure this question must have been raised by somebody already, so maybe you can share a link with us why this hasn't been considered?

Of course, users do need visibility into their dependencies as well as control over what versions they use. But they must also trust and rely on the sources that upgrade their dependencies and make sure renovate does not suggest wrong or potentially insecure revisions: E.g consider the case of #11044, where a tag on every branch is suggested, while pre-commit autoupdate only suggests tags on the default branch. If a project is not set-up very carefully, this could easily introduce a security risk as unverified and unreleased tags are suggested and could be auto-merged.

[rarkins]

Running third party tools to fetch versions is often slow. And they usually only give you the "latest" version, while Renovate users expect the ability to control which version they update to. E.g. "upgrade" might result in a minor version upgrade while the user wants the option of a patch update being taken first. We learnt this from hard experience and unlikely to go back.

Pre-commit is the only manager we've seen with the concept of "tags, but only on default branch". We need someone to look into whether such a filtering can be performed efficiently and not require traversing git commits.

Please don't throw around terms like "security risk" with no basis. If you're referencing a repository you don't control then it's already a security risk. If you insist to go down that rabbit hole then it will be end of conversation. Either you're wrong, or it's irresponsible disclosure and either way will be removed.

[asottile]

here's an example of renovate being a thorn in my side for its pre-commit integration: https://github.com/shellcheck-py/shellcheck-py/issues/32

[electriquo]

@asottile Your hostile and offensive behavior is far from being cool!

not our problem that renovate is bad ... if they don't support 'v' as a prefix on tags then their stuff is going to be broken for basically everything for pre-commit.

Rather than complaining on renovatebot in pre-commit PR's, you should strive to assist in solving issues (pre-commit and renovatebot wise) or just hold your peace. Such behavior makes others fork a project and not contribute to the original source or seek other alternative - is that what you wish for pre-commit?! (This is a rhetorical question, so save me your answer, opinion and ridiculous behavior).

dig https://docs.renovatebot.com/modules/manager/pre-commit/

[HonkingGoose]

Please be kind to each other, and remember there's a human with feelings on the other side of the line.

[skycaptain]

Renovate already did fork the essential logic from pre-commit and decided not to plug/interface/wrap/whatever pre-commit. Also, renovate does not provide such an interface to make it easier for package manager developers to support renovate. Hence, support and commitment can only be expected from renovate's community, but not taken for granted from pre-commit's community (if taking support for granted is even possible for OSS communities). As I wrote in my previous comment re-implementing things comes at a cost. One can not copy someone else's work and then blame the other person or expect them to help when it doesn't work. If that's the spirit, I fear not for the community of pre-commit, but for that of renovate.

@HonkingGoose thank you for this important message. Playing the blame game does not help any of us.

Let's focus on the issue we have to solve to make this thing work. If @asottile can not help here, that's fine; he's busy making pre-commit even better.