`wrapperVersion` property gets removed when upgrading to Maven Wrapper 3.3.3

[breun]

Affected version

3.3.3

Bug description

When I install a newer version of Maven Wrapper, wrapperVersion is removed from .mvn/wrapper/maven-wrapper.properties.

To reproduce (using Maven 3.9.11 as mvn):

mkdir reproducer
cd reproducer

# Install Maven Wrapper 3.3.2
mvn wrapper:3.3.2:wrapper -Dmaven=3.9.11

# This show `wrapperVersion=3.3.2`
grep wrapperVersion .mvn/wrapper/maven-wrapper.properties

# Update to Maven Wrapper 3.3.3
./mvnw wrapper:3.3.3:wrapper

# This returns nothing, because wrapperVersion has been removed, which I don't expect to happen
grep wrapperVersion .mvn/wrapper/maven-wrapper.properties

Note: previously this description also mentioned distributionSha256Sum getting removed during a Maven Wrapper update. I've split this off to its own issue: #367

[slawekjaranowski]

done in:

• [MWRAPPER-143] - The wrapperVersion property is not used in only-script mode #145

[slawekjaranowski]

in order to preserver distributionSha256Sum we should read existing properties

[breun]

done in:

• [MWRAPPER-143] - The wrapperVersion property is not used in only-script mode #145

Update automation tools (and humans too, I guess) need a reliable way to determine the current version of Maven Wrapper. The wrapperVersion property was a way, although admittedly flawed, because I encounter a lot of projects with wrapperVersion bumped manually, but the mvnw* scripts from a much older version, because they were not updated along with the property.

I saw that Renovate plans to determine the Maven Wrapper version based on a regex check on the scripts themselves (renovatebot/renovate#37779), which when matched should be more reliable than reading wrapperVersion. But the regex check will only work as long as the format of the comment line it’s parsed from doesn’t change, so it’s still a fragile method.

Could Maven Wrapper provide its version in a more robust (machine-readable) way?

[GreyTeardrop]

Reposting some context: the wrapperVersion property was added in scope of MWRAPPER-134 by #135.

[breun]

@slawekjaranowski It seems this issue is now about two things:

• distributionSha256Sum getting removed when updating, which you already committed a fix for.
• wrapperVersion getting removed when updating, which was done somewhat intentionally, but maybe without regard for the fact that it was used as a way (but maybe not the best or most robust way, because not all users seem to understand that it was basically a ‘read-only’ property) for tools and people to determine the Maven Wrapper version used by a project, and the discussion on how to best deal with that.

I think it might be good to have separate issues for these two different concerns?

[slawekjaranowski]

@slawekjaranowski It seems this issue is now about two things:

• distributionSha256Sum getting removed when updating, which you already committed a fix for.
• wrapperVersion getting removed when updating, which was done somewhat intentionally, but maybe without regard for the fact that it was used as a way (but maybe not the best or most robust way, because not all users seem to understand that it was basically a ‘read-only’ property) for tools and people to determine the Maven Wrapper version used by a project, and the discussion on how to best deal with that.

I think it might be good to have separate issues for these two different concerns?

exactly, PR are welcome ....

Please let me know if someone would to work on it.

I can release next version at next week 😄

[breun]

@slawekjaranowski I think for now #145 should just be reverted, because wrapperVersion was explicitly introduced as a way to determine the Maven Wrapper version (#249), and removing it has now broken update workflows. Do you agree?

[slawekjaranowski]

@slawekjaranowski I think for now #145 should just be reverted, because wrapperVersion was explicitly introduced as a way to determine the Maven Wrapper version (#249), and removing it has now broken update workflows. Do you agree?

we can simply revert #145 - in one exclusions - I would like to preserve assertions for missing wrapperUrl

[breun]

@slawekjaranowski I think you mean like this?