bumpVersion for maven generates an invalid (pom.) xml

[NiasSt90]

How are you running Renovate?

Mend Renovate hosted app on github.com

If you're self-hosting Renovate, tell us what version of Renovate you run.

No response

If you're self-hosting Renovate, select which platform you are using.

No response

If you're self-hosting Renovate, tell us what version of the platform you run.

No response

Was this something which used to work for you, and then stopped?

I never saw this working

Describe the bug

I've created a Repository to reproduce this bug.

The generated xml looks strange, from:

        <keycloak.version>12.0.1</keycloak.version>
        <springdoc-openapi.version>1.5.7</springdoc-openapi.version>

it generates this invalid xml, the second property was lost completely:

        <keycloak.version>19.0.3</k19.0.3</springdoc-openapi.version>

The generated and broken commit can be found here.

Relevant debug logs

Logs

``` Copy/paste the relevant log(s) here, between the starting and ending backticks ```

Have you created a minimal reproduction repository?

I have linked to a minimal reproduction repository in the bug description

[PhilipAbed]

close this after reading docs and verifying that's the issue, we have been there before https://docs.renovatebot.com/getting-started/installing-onboarding/#self-hosting-on-windows

[PhilipAbed]

see this for reference https://github.com/renovatebot/renovate/discussions/18521

[NiasSt90]

@PhilipAbed

1. there is no windows involved here (debian local and hosted on github).
2. all updates without bumpVersion works fine.
3. My $HOME gitconfig already contains autocrlf = input.

[github-actions[bot]]

Hi there,

Get your issue fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible.

Before we start working on your issue we 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.

We may close the issue if you, or someone else, haven't created 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

[PhilipAbed]

@NiasSt90 seems another issue then, thanks for reporting

[NiasSt90]

After a short debugging round i've found out that Package.fileReplacePosition would be invalid after the (first) bumpVersion was processed.

[NiasSt90]

The problem comes from implicit "grouping" and/or use of a property for the version.

In the example pom it starts to break from:

...
  <version>1.2.0-SNAPSHOT</version><-- bumpVersion removes the -SNAPSHOT on first run
...
 <properties>
...
    <keycloak.version>12.0.1</keycloak.version>
...
 </properties>

//AT LEAST MORE THEN ONE USE OF THE keycloak.version property
  <dependency>
              <groupId>org.keycloak</groupId>
              <artifactId>keycloak-spring-security-adapter</artifactId>
             <version>${keycloak.version}</version>
  </dependency>
  <dependency>
              <groupId>org.keycloak</groupId>
              <artifactId>keycloak-authz-client</artifactId>
             <version>${keycloak.version}</version>
  </dependency>
...

This results in two dep-upgrades for the same keycloak.version property. But with bumpVersion enabled after the first call to bumpPackageVersion() the file-position will be wrong for the second call to updateDependency().

I'm not sure how this loop in lib/workers/repository/update/branch/get-updated.ts:35 should ever have worked successful for more than one upgrade in the same (pom.xml) file.... After the first change the fileReplacePosition could be invalid for all further changes.

For pull-request grouping this would be broken always...

[NiasSt90]

even without bumpVersion activated this could goes wrong if the version-replacement string for one update has a different length then the existing one. Because the position for the next upgrade then could be wrong.

Please correct me if i'm overseen something, but currently all grouping features (implicit with version-property like above) or explicit with pr-grouping are broken for maven projects.

Okay, found it. They are sorted from last position to first, therefore it works without bumpVersion.

[PhilipAbed]

@NiasSt90 i tried to reproduce using your minimal reproduction repo, but i can't look up in nexus.onesty-tech.de it's a private repo, the issue doesn't reproduce, please provide a valid minimal reproduction repo.

im getting this log, and only 2 pull requests unrelated to the problem

DEBUG: Looking up org.keycloak:keycloak-spring-security-adapter in repository https://nexus.onesty-tech.de/repository/maven-public/
DEBUG: dns lookup error
{
  "host": "nexus.onesty-tech.de",
  "err": {
    "errno": -3008,
    "code": "ENOTFOUND",
    "syscall": "getaddrinfo",
    "hostname": "nexus.onesty-tech.de",
    "message": "getaddrinfo ENOTFOUND nexus.onesty-tech.de",
    "stack": "Error: getaddrinfo ENOTFOUND nexus.onesty-tech.de\n    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:109:26)"
  }
}

[github-actions[bot]]

Hi there,

Get your issue fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible.

Before we start working on your issue we 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.

We may close the issue if you, or someone else, haven't created 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

[NiasSt90]

here are two new and absolut minimal repos

• v1
• v2

the broken pull-requests are already generated.

As i already mentioned, bumpVersion is broken by design for maven.

Because calling bumpVersion during the dependency upgrade for-loop (even if the upgrades are sorted from last-file-position to first) can't ever work for maven. Because the version to bump stays always at top of the file. Every next dependency-upgrade will fail.

[rarkins]

@NiasSt90 dramatic statements are unnecessary, please keep it relaxed. I will soon hide all comments which include unnecessary statements, even if they also include other information which is relevant. Please edit any above which are incorrect or unnecessarily exaggerating or dramatic

[rarkins]

Questions:

1. Does the invalid result only occur when more than one dependency gets updated in the same file?
2. Only when they are the same variable?
3. Only when the length of before and after are different?
4. Only when the length of previous version and new bumped version are different?

So e.g. if the bumped version in the file goes from 1.0.0 to 1.0.1 or 2.3.4 it's fine, but if it changes its string length (including e.g. to 1.0.10) then it's not?

To address this, we may need to make changes like:

• Always replace from bottom to top
• Generate the "replace at" index from the current file (post-update) and not the original

[PhilipAbed]

i confirmed reproduction on v1

[NiasSt90]

Questions:

1. Does the invalid result only occur when more than one dependency gets updated in the same file?

yes, as i stated already more then once. the loop should never call bumpVersion for maven if it contains more then one element.

2. Only when they are the same variable?

no, as you see in v2.

3. Only when the length of before and after are different?
4. Only when the length of previous version and new bumped version are different?

yes, because the file-offsets are invalid for the next dependency upgrade. the version which is bumped is always at top of the file.

So e.g. if the bumped version in the file goes from 1.0.0 to 1.0.1 or 2.3.4 it's fine, but if it changes its string length (including e.g. to 1.0.10) then it's not?

To address this, we may need to make changes like:

• Always replace from bottom to top

the array of upgrades was already sorted if i'm looked correct, but only for the dependency-upgrades It don't know that bumpVersion can change at another (earlier) position.

• Generate the "replace at" index from the current file (post-update) and not the original

in my opinion the only good solution is to call bumpVersion after all dependency-upgrades where done. And for maven (and perhaps other managers) this needs/should to be done only once for the project and in the maven case by calling mvn --set-version .... That's why i calling this was broken by design. BumpPackageVersion behaves like a project-level-feature, not a "dependency" feature. Calling bumpPackageVersion for each dependency looks weird to me. I don't understand why this was done this way.

For maven it's unnecessary or can results to unexpected errors (multi-module projects) to replace file-positions in all pom.xml files found in the project manually. Other problem: change from SNAPSHOT to non-Snapshot is really problematic too, but there's already an issue.

And for other platforms this could go wrong to, the sorted list (last-file-position to first) for the dependency-upgrades should only upgrade the dependencies. Calling in the same loop for a second "feature" like bumpVersion without knowledge that it can change at an out-of-order file-position is an defect. You can workaround this by searching again for the possibly new file positions in each loop but this would not solve the original issue as described above.