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

fix: only update lock files if necessary #13462

Closed
wants to merge 6 commits into from

Conversation

secustor
Copy link
Collaborator

@secustor secustor commented Jan 10, 2022

only update lock files if necessary when using update-lockfile… and missing updateLockDependency

Changes:

Detect if the artifacts are already up to date if running not in lockfileMaintenance mode and the manager does not export a updateLockDependency function

Context:

Closes #13363

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 tick 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

https://github.com/secustor/renovate-flapping-grouped-terraform-updates

@secustor secustor requested a review from viceice January 10, 2022 12:25
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.

Otherwise LGTM

lib/workers/branch/get-updated.spec.ts Outdated Show resolved Hide resolved
@secustor secustor requested a review from viceice January 10, 2022 18:38
@viceice viceice requested a review from rarkins January 10, 2022 19:05
(await getFile(
file.name,
reuseExistingBranch ? config.branchName : config.baseBranch
)) !== file.contents
Copy link
Member

Choose a reason for hiding this comment

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

Wait, the contents can be a buffer (Gradle wrapper) or directory (git submodule).

So that needs to be handled too.

@rarkins
Copy link
Collaborator

rarkins commented Jan 10, 2022

I have personal painful experience that modifying this type of logic can backfire, so I'd like to review it very carefully before merging

@secustor secustor requested a review from viceice January 28, 2022 15:48
if (
file?.type === 'addition' &&
file.contents !== '' && // only check if artifact is non-empty --> ignore git-submodule artifacts
(await getFile(
Copy link
Member

Choose a reason for hiding this comment

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

I still think we get into trouble when the file is the gradle wrapper jar. Which is binary.

@rarkins
Copy link
Collaborator

rarkins commented Jan 29, 2022

How many managers do we have left which support update-lock file but which don't have an updateLockedDependency function?

@secustor
Copy link
Collaborator Author

How many managers do we have left which support update-lock file but which don't have an updateLockedDependency function?

  • update-lockfile = Update the lock file when in-range updates are available, otherwise replace for updates out of range. Works for bundler, composer, npm, yarn, terraform and poetry so far

Terraform is the only one

@rarkins
Copy link
Collaborator

rarkins commented Mar 25, 2022

Sorry, but I'd prefer to avoid this branch worker change only for terraform

@rarkins rarkins closed this Mar 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 25, 2022
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.

Terraform provider: flapping branch state when combined with other updates
3 participants