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

feat(bazel-modules): support single_version_override #22610

Merged
merged 13 commits into from
Jun 15, 2023
Merged

feat(bazel-modules): support single_version_override #22610

merged 13 commits into from
Jun 15, 2023

Conversation

cgrindel
Copy link
Contributor

@cgrindel cgrindel commented Jun 6, 2023

Changes

  • Add support for single_version_override in bazel-module manager.

Context

Related to #13658.
Related to cgrindel/renovate_bzlmod_support#19.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure those should be fully ignored? a user can't override that via config. 🤔

@cgrindel
Copy link
Contributor Author

cgrindel commented Jun 7, 2023

are we sure those should be fully ignored? a user can't override that via config. 🤔

Sorry. I don't understand your question.

The single_version_override declaration is slightly different from the other overrides. Here is the top line from the documentation:

Specifies that a dependency should still come from a registry, but its version should be pinned, or its registry overridden, or a list of patches applied.

@cgrindel cgrindel requested a review from viceice June 7, 2023 13:28
@cgrindel
Copy link
Contributor Author

cgrindel commented Jun 9, 2023

@viceice Gentle ping. Could you elaborate on your comment?

are we sure those should be fully ignored? a user can't override that via config.

@viceice
Copy link
Member

viceice commented Jun 10, 2023

@viceice Gentle ping. Could you elaborate on your comment?

are we sure those should be fully ignored? a user can't override that via config.

if you set a Skip reason, renovate will always ignore that dependency. there is no way for a user to override that. so renovate can never update that dependency. so a user always needs to manually do it.

if the override only changes the registry, we should simply add that to the original dep and ignore the override.

if the override defined a version, it should be updatable by renovate. we can return enabled=false, so they are disabled by default, but can be enabled by user via package rule.

@cgrindel
Copy link
Contributor Author

if you set a Skip reason, renovate will always ignore that dependency. there is no way for a user to override that. so renovate can never update that dependency. so a user always needs to manually do it.

That is exactly the workflow that we want, in this case. If the client specifies a version in single_version_override, they are pinning the version. We do not want to update that value. There is no other reason to specify the version in single_version_override.

if the override only changes the registry, we should simply add that to the original dep and ignore the override.

Agreed. That is what this PR does. The test case for this scenario is here.

@viceice
Copy link
Member

viceice commented Jun 12, 2023

@rarkins @secustor @JamieMagee WDYT?

@secustor
Copy link
Collaborator

This seems like the same scenario as required_version of the Terraform manager.

Usually developers use that feature to limit Terraform version avaiable to module users, tough we still update them under a different depType to allow an easy way to disable that feature.

@rarkins rarkins changed the title feat: support single_version_override in bazel-module manager feat(bazel-modules): support single_version_override Jun 13, 2023
@rarkins
Copy link
Collaborator

rarkins commented Jun 13, 2023

Seems ok to leave it as-is for now, maybe in future users will want a way to have it updated

@cgrindel
Copy link
Contributor Author

@viceice @rarkins I have not seen a comment from @JamieMagee. Is it possible to continue the review on this PR? My next PR adds documentation and enables the bazel-module manager. I would like to get this merged and in production before I disappear on vacation in July.

@viceice viceice added this pull request to the merge queue Jun 15, 2023
Merged via the queue into renovatebot:main with commit ad61b6c Jun 15, 2023
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 35.119.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@cgrindel cgrindel deleted the feat/13658_bazel_module_single_version_override_alt branch June 15, 2023 16:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants