Modified the VB.NET Template so that GitVersionInformation is in the Global namespace

[odalet]

Description

• Modified the VB template so that the resulting class is part of the global namespace
• Tweaked a bit GitVersionInfoGenerator.cs so that it uses different tab lengths for the VB case and the F#/C# cases (this is simply a cosmetic change so that VB output formatting is correct).
• Modified msbuild-task.md in the documentation so that:
  • It does not try to find GitVersionInformation in the assembly's default namespace but in the global namespace
  • It works with F# generated classes (properties vs fields).

Related Issue

Fixes #2310

Motivation and Context

Now, the VB version of GitVersionInformation belongs to the Global namespace in the same way as its F# and C# counterparts.

How Has This Been Tested?

• Fixed the units tests
• Double-checked with ILSpy that the resutling class is indeed part of the global namespace. Here is what it looks like when decompiled with ILSpy:

// GitVersionInformation
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

[CompilerGenerated]
[ExcludeFromCodeCoverage]
internal sealed class GitVersionInformation
{
	public static string Major = "1";

	public static string Minor = "2";

	public static string Patch = "3";

	public static string PreReleaseTag = "unstable.4";

       ...

	public static string CommitsSinceVersionSourcePadded = "0005";

	public static string CommitDate = "2014-03-06";

	private GitVersionInformation()
	{
	}
}

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[asbjornu]

Minus the failing dotnet format check and a potential ILMerge issue, I think this looks good.

[asbjornu]

I was just thinking, if GitversionInformation is moved to the global namespace, won't this make ILMerging difficult? If several of the merged assemblies are versioned with GitVersion, we would potentially end up with several colliding GitversionInformation classes?

[odalet]

That poses no problem because the generated classes are, and this is very important, internal. An external assembly cannot see these class unless you use reflection, and when using reflection they are resolved with their full name (which happens to be only the class name) against a specific assembly, therefore, no collision.

This trick, I often use with Visual Studio's "shared projects": they are just bunches of code that get "included" into the consuming project. I always make sure classes in these shared projects are internal so that they can be used from any assembly even if these assemblies reference one another.

[odalet]

And if this were an issue, it would already have shown with C# or F# generated classes that are already part of the global namespace (and internal).

[odalet]

Mmmh, maybe I spoke too quickly, I missed the bit about ILMerge... Everything being part of the same assembly in the end, it may be an issue. I'll give it a try, and sorry if you thought I was teaching you how visibility worked in .NET! However, my point that the issue already exists if any is still valid.

[odalet]

Well, after a quick googling, it appears this indeeds poses problems:

Suppose class A is internal to App, and also to one of the lib (with same namespace). Before merging there will not be any issue. After merging it will become issue to resolve ambiguous reference.

See https://stackoverflow.com/a/14042227/107552

[odalet]

And with Fody + Costura, everything just works out of the box. This is because, the principle is radically different. The assembies' IL is not merged, instead the dependent assemblies are embedded as resources in the main assembly, then extracted and loaded at runtime (during module initialization). Thus, the assemblies never cease to exist as such and type resolution just works.

[odalet]

If you are interested in witnessing the tests I did, everything is in this repository: https://github.com/odalet/GitVersionTests

[odalet]

And now for ideas on how this whole "code generation" thing could be tweaked:

• The generated class could be put in a namespace or have a name that will guarantee non-collision. To guarantee this, the class name should be based on the assembly name (and not on some 'default' or root namespace). The class could be inside a namespace named as the assembly or be an inner class inside a class named like the assembly or be a concatenation of assembly name and GitVersionInformation...

• A property / command line argument could be provided to let the user choose the name of the generated class.

• I recently stumbled upon the GitInfo. Its purpose is similar to GitVersion's with the following differences:

  • Completely MSBuild-based, no C# code at all
  • Seems to have far less feature. It seems to simply expose Git information and does not attempt clever inference of versions.
  • There is however one feature that may be interesting here:

It, too, generates code containing version information. However:

• The version information is made available through constants, not properties or fields.
• The file containing this information is generated soon enough for it to be available at "design-time": it can be used without resorting to reflection.

For example, here is what I can write when using GitInfo:

Console.WriteLine($"{ThisAssembly.Git.SemVer.Major}.{ThisAssembly.Git.SemVer.Minor}.{ThisAssembly.Git.SemVer.Patch}");

The advantage of this strategy is that because constants are inlined in the calling code, the original type that exposes them is useless at runtime and therefore, collisions on this type are not anymore an issue. And being able to use the version information without reflection also seems more user-friendly.

So, mixing some of these ideas may be the way to go. However, in any case, it is quite some work, and most probably breaking. In the meantime, my experiments with a selection of assembly merging tools show that the collision problem can be mitigated.

It's now up to you to reflect upon all this and decide what or what not to do :)

Hope this helped!

[asbjornu]

Constants are a great idea for how we can implement this in a new major version of GitVersion! Whether we'll do this for v6 or not is mainly dependent on timing; if someone submits a pull request implementing this before we go live with v6, it will be released with v6. If not, we would have to postpone it to v7.

[odalet]

Generating constants should be rather straightforward. The MSBuild part that generates this soon so that the user can call into it may be a little trickier (I'm not very proficient at MSBuild ;)).

I'll probably come up with an issue that will serve as a discussion and specification of what can be done.

[odalet]

Yes I noticed this dotnet format issue (and same problem with PR #2314), but honestly I don't know what to do with this. It also seems to be related to files I didn't change...

[asbjornu]

If you run dotnet format locally, it should fix any problems that may be. I agree the files it complains about are unrelated to your PR, but please just try anyway.

[odalet]

Will do this (but not before the WE). Week days are dedicated to software I'm paid for ;) I'll also experiment with merging assemblies.

By the way, I still have one question: do you want me to rebase/squash everything in a single commit that I'd force push or are several commits acceptable?

[asbjornu]

Will do this (but not before the WE). Week days are dedicated to software I'm paid for ;) I'll also experiment with merging assemblies.

👍

By the way, I still have one question: do you want me to rebase/squash everything in a single commit that I'd force push or are several commits acceptable?

Several commits are fine, but if you can avoid merge commits and do rebase instead (to stay up to date with master), that would be great.

[odalet]

Got it!

[odalet]

Rebasing atop master had the dotnet format check pass although, now the Azure DevOps macOS tests fail or rather the test runner crashes...

[asbjornu]

I think we can get away with calling this a bugfix, even though it may break something for someone.

[asbjornu]

Thank you so much for your contributions, @odalet! 🙏