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: use correct values for replacements #18858

Merged
merged 2 commits into from
Nov 10, 2022

Conversation

t-kulmburg
Copy link
Contributor

@t-kulmburg t-kulmburg commented Nov 10, 2022

Changes

Changes the replacement updates to use replacementName and replacementVersion from config instead of dependency

Context

This was part of my other PR.

Reason for this change is that I encountered a problem when replacing 2 different versions of the same dependency with different new dependencies.

I also tested this in my big replacement demo repository to ensure it doesn't break other replacements when combined with my other PR, which contains the main replacement feature.

Example

Dockerfile:

FROM openjdk:8-jre
FROM openjdk:17-slim-buster

packageRules:

    {
    "matchManagers": ["dockerfile"],
    "matchPackageNames": ["openjdk"],
    "matchCurrentVersion": "/^17/",
    "replacementName": "eclipse-temurin",
    "replacementVersion": "17-jdk-jammy"
    },
    {
      "matchManagers": ["dockerfile"],
      "matchPackageNames": ["openjdk"],
      "matchCurrentVersion": "/^8/",
      "replacementName": "eclipse-temurin",
      "replacementVersion": "8u342-b07.1-jre"
    },

Log output:

logger.debug(`CONFIG: ${config.replacementName} -- ${config.replacementVersion}`)
logger.debug(`DEP: ${dependency.replacementName} -- ${depName.replacementVersion}`)
DEBUG: CONFIG: eclipse-temurin -- 8u342-b07.1-jre (repository=xx/renovate-test)
DEBUG: DEP: eclipse-temurin -- 8u342-b07.1-jre (repository=xx/renovate-test)

DEBUG: CONFIG: eclipse-temurin -- 17-jdk-jammy (repository=xx/renovate-test)
DEBUG: DEP: eclipse-temurin -- 8u342-b07.1-jre (repository=xx/renovate-test)

This shows that when using dependency, both will get replaced with the same version.
When using config however, we get the replacement we wanted.

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

@astellingwerf
Copy link
Collaborator

How can it be that this fix does not impact any test?

@viceice viceice self-requested a review November 10, 2022 15:02
@astellingwerf
Copy link
Collaborator

I think you're possibly fixing #14962 with this, but we're still hiding the fact that replacements are implement in the wrong/a illogical place.

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.

Looks good, we should try to add tests to not break in future

@rarkins rarkins enabled auto-merge (squash) November 10, 2022 19:11
@rarkins rarkins merged commit 569f5fb into renovatebot:main Nov 10, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 34.22.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@t-kulmburg t-kulmburg deleted the fix/take_correct_replacements branch November 11, 2022 13:44
@JordiVM

This comment was marked as off-topic.

@viceice
Copy link
Member

viceice commented Nov 22, 2022

@JordiVM That is unrelated to this PR. Please open a new Discussion for futher help.

@renovatebot renovatebot locked as resolved and limited conversation to collaborators Nov 22, 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.

Replacements cause ignoring prerelease/compatibility
6 participants