add MinVerVersionOverride

[adamralph]

Closes #180.

[adamralph]

@bording would you mind taking a glance at this?

There are two reasons for passing the version override into the CLI (which requires the conditionals since it only makes sense in the context of the MinVer project):

• The properties which extract the various segments from MinVerVersion rely on the version being well formed. If someone were to set MinVerVersionOverride to a nonsense value, they'd end up with obscure MSBuild errors pointing to lines in the MinVer targets file.
• To keep the targets file simple. If the override was implemented in the targets file, we'd have to go back to multiple conditional targets.

[bording]

I don't really see the harm in letting the console app always have the --version-override option. It simplifies things if it's not conditional, and maybe there's a use case for it to be there. If someone did use it, they'd still have to pass a well-formed version into it, so I can't see how it would actively harm things to keep it around.

As to the larger feature here, reintroducing a way to override the versioning logic, this doesn't seem to do anything to address the problem we ran into when we had this before: If someone sets MinVerVersionOverride as an MSBuild property, then the environment variable you're suggesting will be ignored.

I guess the reasons for using the override are slightly different than what was proposed for the previous feature, meaning it would be less likely for that to happen, but it still could be a problem.

There is another approach to consider, though I'm not sure it's worth the effort or not. When the versioning is invoked, it could write the calculated version to disk, using the commit sha as the file name. Then when it is invoked in the future, it could first check and see if that file exists, and if it does, use that version instead.

This is what GitVersion does. It creates a gitversion_cache folder inside the .git folder and writes its files in there.

[adamralph]

@bording thanks for chiming in.

Can you think of a use case for the version override in the CLI tool?

I'm not really bothered about the property vs env var thing. It's the same for all the options. It would be weird to specify MinVerVersionOverride as a property, because that would effectively remove the need for MinVer. It should never be required as an env var on a build server either, since that would imply that tagging the repo doesn't work, and that would mean there is something catastrophically wrong with MinVer.

Perhaps I'm missing something, but I'm not sure I understand what benefit the disk cache would provide. If I ran minver -m 2.0 and got back 2.0.0-alpha.x and then I ran minver -m 3.0, I'd expect 3.0.0-alpha.x, not a cached 2.0.0-alpha.x. Similarly, if I ran minver, then tagged HEAD, then ran minver again, I'd expect the version in the tag, rather than a cached version from the first run. That means the cache would have to be based on the inputs and the tags, which defeats its purpose. I may as well just run the versioning logic each time.

[bording]

Can you think of a use case for the version override in the CLI tool?

I don't have a specific use case in mind, but I also don't see how any existing use cases are harmed by having it be there.

I'm not really bothered about the property vs env var thing. It's the same for all the options. It would be weird to specify MinVerVersionOverride as a property, because that would effectively remove the need for MinVer. It should never be required as an env var on a build server either, since that would imply that tagging the repo doesn't work, and that would mean there is something catastrophically wrong with MinVer.

It's true that there's not a great reason to do it, so the approach in this PR does seem a like a solution to the issue.

Perhaps I'm missing something, but I'm not sure I understand what benefit the disk cache would provide. If I ran minver -m 2.0 and got back 2.0.0-alpha.x and then I ran minver -m 3.0, I'd expect 3.0.0-alpha.x, not a cached 2.0.0-alpha.x. Similarly, if I ran minver, then tagged HEAD, then ran minver again, I'd expect the version in the tag, rather than a cached version from the first run. That means the cache would have to be based on the inputs and the tags, which defeats its purpose. I may as well just run the versioning logic each time.

Given the way things currently work, this is valid, but I'm wondering if that's a sign of another problem. Looking at GitVersion, which also has a command line, and MSBuild integration, the way that is handled is that there is an external configuration file (GitVersion.yml) that both of them look at for their settings, making it easy to keep them in sync.

They also have the idea that a tag on a commit could alter the calculation that was previously cached.

---

Thinking about this a bit more, if using some external config & cache isn't desirable (and at this point I would say it's not something worth doing), then I wonder if there's not a simpler way to achieve this?

If you're using the commandline tool to version things, then what is the benefit of having the MinVer MSBuild integration installed in the first place? Seems like you could just invoke the tool, and then set Version in an env var directly, and then your project would pick that up and use it.

[adamralph]

If you're using the commandline tool to version things, then what is the benefit of having the MinVer MSBuild integration installed in the first place? Seems like you could just invoke the tool, and then set Version in an env var directly, and then your project would pick that up and use it.

To replicate the behaviour of the MinVer package, you'd also have to set AssemblyVersion, FileVersion, and PackageVersion, and that would require splitting out the major, minor, and patch parts.

[bording]

@adamralph You wouldn't need PackageVersion since Version would be set early enough that it would be used for PackageVersion, but yeah the others would a bit more tricky.

Overall, the approach in this PR seems to work. I guess it just feels more like a workaround to me than the way you'd actually want it to work.

However, the only other way I can think of at the moment would be for the tool to read settings from a shared config file.

[adamralph]

@bording I'm continuing to give this some thought. The thing that puts me off the external config file approach is that the canonical usage of MinVer is the MinVer package, and there's no reason for that to use an external file when it can just use MSBuild properties. The CLI tool was really just an extra, thrown in because someone else wanted it. That said, I've found it useful myself for testing MinVer on arbitrary repos without installing the MinVer package and performing a build. But I'm reluctant to introduce an external config file just because of the CLI tool. A version override property feels more palattable, because there could be other use cases for it, and reading it is just like reading any other setting.

[bording]

. The thing that puts me off the external config file approach is that the canonical usage of MinVer is the MinVer package, and there's no reason for that to use an external file when it can just use MSBuild properties. . The CLI tool was really just an extra, thrown in because someone else wanted it.

As long as the CLI tool is positioned as an extra, then I agree that an external config file doesn't make sense.

However, if you wanted to change that and make it more of a 1st-class consideration, then I think it could make sense to go down that route.

Perhaps both tools continue to operate as they do now, but they could both opt-in to looking at a config file instead? The CLI could gain a --use-config-file option and the MSBuild tool could get a UseConfigFile property. That way you could ensure both tools produce the same version when you are in a scenario where that matters, and don't need to worry about which order they are invoked to make sure an env variable is set.

On the other hand, maybe that's just overkill, and what this PR is doing is enough.

[adamralph]

@bording I've been agonising over this for a few days but I think I'm going to YOLO it and merge.

FYI there was a conversation on Slack about multi-container builds, where the version is figured out once and then used in several containers for various parts of a build process. This environment variable would also be very useful there. The alternative would be ensuring that the .git folder (and config file) are either mounted or copied to each container, so the env var feels like a much cheaper way to do it.

Also, I stumbled upon another use case myself: I wanted to give someone access to the build from this PR via myget, so I altered the version suffix env var on the build server, rebuilt the PR, and deployed it to myget. For another project, which uses MinVer, I would have had no way to do that without this override. I guess, while I'm using Appveyor, I could have pointed them to the CI NuGet feed, but if using Travis CI or some other CI system, there would be no such thing. Also, I could customise the versioning for PR's as described in the README, but if I only release PR builds once in a blue moon, I don't really want to add that complexity.

This is leading me to the conclusion that this override may have all sorts of use cases other than just aligning the CLI tool and the MinVer package, so I think it makes sense to have it anyway, as a kind of Swiss Army Knife feature. We still have the option to introduce the config file later if required, as a separate feature.

[bording]

@adamralph Given that there are additional use cases, then it seems like it's worth bringing in, then. Like you say, a more unified approach could always be introduced later.

[adamralph]

Released in beta 3.