[Azure DevOps] Leave the original Build Number when no GitVersion variable in Build Number

[kirkone]

The Build number should not replaced when there is no GitVersion variable in it.

This is discussed in #1457

[tests-checker]

Could you please add tests to make sure this change works as expected?

[kirkone]

Could you please add tests to make sure this change works as expected?

I can not find a place where to place these tests. Any help?

[kirkone]

I it possible to get a review on this PR?

[arturcic]

Sorry for not coming back. To be honest the idea to disable BuildNumber override looks good to know, but I'd prefer to have a more consistent approach, we need to add this ability to all supported CICD not only Azure pipeline, what do you think?

[kirkone]

As far as I can see the other pipelines do not behave like this already.
This seems to be only in the code for Azure DevOps.
In AzDO the BUILD_BUILDNUMBER environment variable is also used as the name of the build.
When I use a build name "My Awesome Build" in the current version of GitVersion this will be replaced by "PackageName.1.0.5...".

I can have a look into the other pipelines again and make this sure.

[kirkone]

I do not know the other pipelines enough to understand what setting the Build number will have for konequences.
Maybe the Solution mentioned here is more consistent:
#1457 (comment)

I do not know how to implement such a switch but this would allow to disable the the setting of the BuildNumber completly.

The change I made here is only the leave the BuildNumber untouched when there is no GitVersion variable in it. In my opinion this has no side effects to the other pipelines.

[kirkone]

@arturcic I checked again and I think the behavior of replacing the whole string when no variable is in it is a thing only in the implementation of the Azure DevOps part.

If you still want the current behavior you can use something like that in your build name: "$(GitVersion.SemVer)".

What do you think?

[arturcic]

As far as I can see, these are the CI providers that also override the build number:

AppVeyor - runs a PUT http request to override the build number
ContinuaCi - sets the setBuildVersion using their mechanism of messaging
Teamcity - sets the buildNumber using their mechanism of messaging
VsoAgent (aka AzurePipelines) - set the build.updatebuildnumber using their mechanism of messaging

So as you can see there are at least 4 CIs that by default override the build number. I do agree that VsoAgent does that in it's own way, it's trying to resolve GITVERSION variables in the string, and we should fix that to be consistent with the others.

And I do agree that we need to be able to disable the build number overriding in CIs but that should be done on the BuildServerBase level

[kirkone]

OK, you are right, but can you give me a hint how to implement such a switch?

I also think that the difference ist how the build environment will handle the change of the buildnumber value. What I want to say is that only Azure Pipelines will use the build number also as the name for the build. I asked for splitting this up on their side but this is not possible as far as I know.
The other environments will not behave like this as far as I know.

[asbjornu]

@kirkone, can you please submit a proposal for how you would like this implemented in a least common multiple of what all build servers support? Just a sketch so we have something concrete to discuss, because now it's all a bit too abstract for me to even comment on.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

[arturcic]

Bump

[kirkone]

Hi @asbjornu ,

sorry for the late response.

I think the solution with the least impact on other Pipelines would be to add a new switch for the tool.
Something like --skipbuildnumber. If this is set the code block with the replacement will be skipped.

Aside from that I am very sure that replacing the whole string when there is no GitVersion reference in it is not the way most people would expect. If you want to use the replacement put a variable in the build name.

[arturcic]

@kirkone seems to be a good idea. I would go with the --updateBuildNumber instead, meaning better to explicitly specify you need to update the build number. Or we can better have 2 configurations in the config file

updateBuildNumber - true|false
updateBuildNumberFormat - the build string format

[kirkone]

@arturcic but this would be an breaking change then? So the tool would behave in a different way as before when it is called without the parameter?
Or do you mean the default value when not set is true?

[arturcic]

Yes it looks like a breaking change. What I mean is if you want to enable a feature, better to specify you need to enable it, instead of not specifying. We can include in the release notes it's breaking change

[arturcic]

@asbjornu Thoughts?

[kirkone]

Is there also a way to include this setting in the .yaml file?

[arturcic]

Is there also a way to include this setting in the .yaml file?

That will be the second option. Use config instead of cmdline switch. And I would prefer to have it as 2 configs, one to enable updating, and the other the format

[arturcic]

Still this will be a breaking change

[kirkone]

The format might not be necessary because if you want the replacement to happen then you easily can add variables to your Build name. That is the same way it works now.
I only want to change what happens if there is no variable set in the BuildName.

[arturcic]

Ok, was thinking for all the Build Server we support. Are you in a position to work on a PR in that direction?

[kirkone]

besides the changes I made so far I do not have any idea where to add new config values.
I will have a look into it again and update this PR.

[arturcic]

You can start looking here https://github.com/GitTools/GitVersion/blob/master/src/GitVersionCore/Configuration/Config.cs

[kirkone]

Startet a new approach ...

[kirkone]

I added the configuration value but can not find an example how to access the configuration.

Should I get it from the DI?

[arturcic]

I added the configuration value but can not find an example how to access the configuration.

Should I get it from the DI?

Check this https://github.com/GitTools/GitVersion/blob/master/src/GitVersionCore.Tests/ConfigProviderTests.cs

[arturcic]

@kirkone I have rebased on top of master and fixed/ added some tests. Thank you for the initial work and the thought process

[kirkone]

Thank you @arturcic for finishing this!

I am very happy to see this. Sorry for not getting this ready, but I was not able to get those last changes running on my system.

Again, thanks! I will now prepare all my Pipelines to use this new feature.

[github-actions]

🎉 This issue has been resolved in version 5.3.3 🎉
The release is available on:

• GitHub Release
• Chocolatey Package
• .Net Global Tool

Your GitReleaseManager bot 📦🚀

[kirkone]

hello @arturcic

sorry for coming back to this but I missed a little detail in your changes.
setting update-build-number: false will disable all the output to the build Server. So there are all variables will not be set.

I do not think this is working like it should.

This here:

GitVersion/src/GitVersionCore/Core/BuildAgentBase.cs

Line 33 in 24eba66

if (writer == null || !updateBuildNumber)

should look like:

if (writer == null)
{
    return;
}

if (updateBuildNumber)
{
    writer($"Executing GenerateSetVersionMessage for '{GetType().Name}'.");
    writer(GenerateSetVersionMessage(variables));
}
writer($"Executing GenerateBuildLogOutput for '{GetType().Name}'.");
foreach (var buildParameter in GenerateBuildLogOutput(variables))

Should I provide a new PR?

[arturcic]

Yes please

[kirkone]

Sorry for beeing so late here. I had some trouble in real live.

I created a new PR: #2457

@arturcic can you please have a look?

[arturcic]

Glad you came back, hope you're better now