GH-2127 - allow json and buildserver output at the same time

[arturcic]

Closes #2127 Closes #1236

[asbjornu]

Great stuff! What happens if you duplicate -output json, for instance? Also, it's now impossible to not output JSON, true?

[arturcic]

What happens if you duplicate -output json, for instance?

We use ISet, that means only one instance will be saved.

Also, it's now impossible to not output JSON, true?

We add by default in the constructor of Arguments the json because we need to preserve the way it worked before

[arturcic]

But not using json at all, I think the old behavior allowed that. Let me check

[arturcic]

@asbjornu now it's possible to add only -output buildserver without output json as well as multiple times -output json or -output buildserver or a combination of those

[gep13]

Interesting! This will help with one of the long time issues with using GitVersion and Cake on CI system. Namely that you would have to run GitVersion twice, once to set the BuildServer version and then again to get the asserted variables to use in rest of script.

[arturcic]

Interesting! This will help with one of the long time issues with using GitVersion and Cake on CI system. Namely that you would have to run GitVersion twice, once to set the BuildServer version and then again to get the asserted variables to use in rest of script.

Right, that was one of the reasons I implemented this, to avoid running twice.

[asbjornu]

Brilliant!

[arturcic]

@gep13 I think this change will break cake alias for GitVersion, am I right?

[gep13]

Off the top of my head, I am not sure. I would need to check. Travelling at the minute, so not in a position to look at this just now.

[arturcic]

@gep13 I think this will still work with the cake aliases, just whoever uses the cake aliases will not be able to specify both the json and buildserver output. So I think we can merge this. @asbjornu ?

[asbjornu]

Sure. But just to be clear: This closes #1236 and probably makes #983 obsolete as well, no? Writing the console output to a file is just > GitVersion.json in most console environments.

[asbjornu]

Unless we use the implementation of #983 to fix #1031. 🤔

[arturcic]

I think it will close #1236,
As for #983 just > GitVersion.json might work except when the tool fails it will write the error to the GitVersion.json file so I'd prefer a new output type instead.
As for #1031 it might partially fix the issue as we will only call it once instead of twice, making it less probable, but in the case of GitVersionTask it might occur as for each task it calls the GitVersion (something to refactor I think)

[asbjornu]

Good analysis. Please add Closes #1236 in your PR description so it gets closed automatically on merge and then just merge ahead.