Msbuild: VersionPrefix and VersionSuffix are misnamed

[adamralph]

Prefix:

a word, letter, or number placed before another.

The value of this property never _prefixes the version_. That would imply that the version is another string of characters, and the value of this property precedes it. This never happens.

Until I read the documentation, I was surprised to see values of VersionPrefix of the form "1.0.0", since "1.0.0" is not a prefix to the version. It _is_ the version.

The same argument could be applied to VersionSuffix, depending on the interpretation of "version". If "1.0.0" is the "version" in "1.0.0-beta1" then it's true to say that the "version suffix" is "beta1", but if "1.0.0-beta1" is the "version", then VersionSuffix is similarly misnamed.

---

Update:

Based on the comments below, it seems that the general opinion is that "1.0.0-beta1" is the "version", implying that VersionSuffix is also misnamed. I'm re-titling the issue from "VersionPrefix is misnamed" to "VersionPrefix and VersionSuffix are misnamed".

[tpluscode]

The naming used here should follow the semver, right?

According to that spec, both 1.2.3 and 2.4.5-beta.2 are versions. The latter being a pre-release version in which what you called VersionSuffix is referred to as dot separated identifiers.

GitVersion uses the term pre-release tag although I'm not sure whether they coined that term. @asbjornu?

[asbjornu]

I think @JakeGinnivan might know better than me where pre-release-tag came from, but as discussed in GitTools/GitVersion#1054, we might change tag to label or identifier (as seems to be the word used by semver) to not confuse it with a git tag.

[dasMulli]

fyi the logic comes from https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.DefaultAssemblyInfo.targets#L18-L22

It's hard since you need to be able to construct the Version from two parts. Changing the version based on the presence of a VersionSuffix isn't a good option here because it may be a global property and can't be overwritten.
So i guess the feature request would be to create an alias for VersionPrefix (e.g. VersionNumber - but i'm horrible at naming things)

[adamralph]

Based on the comments above, it seems that the general opinion is that "1.0.0-beta1" is the "version", implying that VersionSuffix is also misnamed. I'm re-titling the issue from "VersionPrefix is misnamed" to "VersionPrefix and VersionSuffix are misnamed".

[adamralph]

Unfortunately SemVer doesn't define terms to describe the "1.0.0" and "beta1" parts of the _version_ "1.0.0-beta1" so I guess we'll have to define them (or look for precedents elsewhere).

But, before going too far down that path, perhaps we should challenge

you need to be able to construct the Version from two parts

Why?

[dasMulli]

you need to be able to construct the Version from two parts

My reasoning is mostly based on the ability and need to pass version-related properties as arguments to msbuild invocations, e.g. from CI.
However, the importance of the different usage scenarios need to be weighted against each other.. Let me elaborate:

You can currently pass /p:VersionSuffix=beta1 and it will generate a 1.0.0-beta1 version (the dotnet cli even has convenience parameters for that). if you set /p:Version=1.2.3;VersionSuffix=beta1, you end up with 1.2.3 - which is expected and you get the same behaviour if you specified these two properties inside a project file or from command line. While not perfectly named, it is at least consistent.

However, take the following project file for example:

<Project>
  <PropertyGroup>
    <Version Condition="'$(Version)' == ''">1.0.0</Version>
    <Version Condition="'$(PreReleaseLabel)' != ''">$(Version)-$(PreReleaseLabel)</Version>
  </PropertyGroup>
  <Target Name="Build">
    <Message Importance="high" Text="Version: $(Version)"/>
  </Target>
</Project>

1. Invoke it with dotnet msbuild /p:PreReleaseLabel=beta1 and it prints 1.0.0-beta1.
2. Invoke it using dotnet msbuild /p:Version=1.2.3;PreReleaseLabel=beta1 and you get 1.2.3, because Version is now a global property and can't be overwritten.
3. Put a <Version>1.2.3</Version> at the top of the property group (pretend everything else is coming from the SDK) and run dotnet msbuild /p:PreReleaseLabel=beta1. The result will be 1.2.3-beta1

My problem with this is that 2. and 3. look similar - both properties set to a value - but behave differently because of the way global msbuild properties work, which is confusing for beginners trying to set up their project / CI.

There are a few workarounds for that but they would require a different property (ResolvedVersion) or each target that needs a version had to look at all available properties. (Not to mention this would be a breaking tooling change).

Summed up, I think it's best to leave Version being composed from two properties, but maybe figure out better names (falling back to the current ones).

[adamralph]

if you set /p:Version=1.2.3;VersionSuffix=beta1, you end up with 1.2.3...

Therein lies the problem. If I pass Foo, and FooSuffix, I expected the value of Foo to be suffixed by the value of FooSuffix. The current behaviour is surprising and does not follow the semantics conveyed by the property names.

• which is expected...

It is only expected after you have experienced the surprising behaviour or read the documentation, and you have retained and recalled that knowledge before the occasion. The names (semantics) cannot be relied on to remind the user of the expected behaviour.

I still haven't seen a defense of the assertion that we need the version to be composed of two properties. What does this offer over a single property named Version?

[asbjornu]

@adamralph:

Unfortunately SemVer doesn't define terms to describe the "1.0.0" and "beta1" parts of the version "1.0.0-beta1" so I guess we'll have to define them (or look for precedents elsewhere).

Yes, SemVer defines them as such:

Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

And:

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version.

So SemVer calls these pre-release strings that are suffixed to the version for labels or identifiers. I would call the entire string for SemanticVersion, the version number for VersionNumber and the pre-release suffix for PreReleaseLabel.

[adamralph]

@asbjornu defining SemanticVersion as {VersionNumber}-{PreReleaseLabel} is not a bad idea. For completeness, I would extend it to {VersionNumber}-{PreReleaseLabel}+{BuildLabel}.

However, I still challenge the need for multiple properties. I haven't seen a strong argument for having multiple properties over a single Version property.

[rainersigwald]

This issue was moved to dotnet/sdk#1131