Stability of MSBuild integration for future .NET Core releases

[stazz]

Hi,

When helping with issue #1503 the concern was arised that any new .NET Core releases have potential to break the UtilPack task factory used by GitVersionTask. This stems from the fact that while it is possible to lock down the NuGet assemblies version on .NET Desktop (and Mono), it is not possible to do so on .NET Core. This because NuGet assemblies are part of Trusted Platform Assembly list when dotnet build command is invoked, and it is not possible to load another version of them by the task factory. This will cause crashes as new versions of .NET Core introduce potentially binary-incompatible changes to the used APIs of NuGet assemblies.

So the root problem can be solved if there is a way to lock down NuGet assembly version on .NET Core build environment. One such way is to use .NET Core Global Tools, since they run using plain dotnet instead of dotnet build.

It seems that you are publishing GitVersionTask already as .NET Core Global Tool, according to #1463 . Maybe the most straightforward way is to just invoke the GitVersionTask as global tool from .targets file, installing it if it is missing from local system? Would it produce all information that is required by the .targets file?

This would essentially split the behaviour of GitVersionTask in MSBuild usage into two parts:

• On .NET Desktop (and Mono) environment, the UtilPack task factory would be continued to be used
• On .NET Core environment, the global tool would be installed if needed, and invoked, parsing the potential output parameters from the output or file. The UtilPack task factory would not be used at all.

[arturcic]

Thanks @stazz for detailed explanation, we might consider the approach you suggested

[dazinator]

Hi @stazz it's definately something to consider.

Couple of points thinking out loud. A csproj file built with dot net would need the global tool installed, but built with msbuild it wouldnt. Today we can build the same csproj file with either, there are no platform specific instructions which is nicer imho but probs not a deal breaker.

If we invoked the global tool as part of the build presumably we would have to pass any msbuild properties needed to gitversion and then get its STDOUT and then set the same msbuild properties during the build that the task sets today. This means the task could be dumbed down, and dependencies removed (presumably we'd need to remove all dependencies because we will no longer have utilpack to restore them for us) but we would still need a task, and now we might need two tasks, one per platform (but hopefully not, hopefully we coukd keep a single task targeting netstandard for this that runs on mono, desktop and core - it was dependency resolution that made this originally non trivial but bow if we dont have any dependencies it might just work..).

We'd have to think whether we need to extend gitversion command line for this mode of operation (i.e to support gitversiontask in its new endeavours)

Arguably if this is workable, we should use a consistent approach on Net Desktop as well, i.e by calling GitVersion.exe inatead of dotnet gitversion global tool - keeping as much of the logic as unified accross platforms and as consistent as possible.

Utilpack msbuild was / is attractive because it lets you write a single msbuild task targeting netstandard that runs everywhere taking care of platform differences = less platform specific divergence in our code base. I'd support any change that is done in a way that keeps platform specific code as light as possible :-)

I have an extension of your idea @stazz what do.you think of this:

1. We still use utilpack task factory, but remove all dependencies / dumb down the task
2. We have a way to prevent utilpack msbuild from.doing a restore - i.e it no longer consumes nuget apis and so we avoid breaking changes under netcore.
3. We bundle gitversion.exe and the gitversion global tool inside the gitversiontask nuget package tools folder.
4. We use your approach of shelling out to these tools within the nuget package. The task could use system.diagbostic.process to run gitversion.exe or dotnet gitversion from within the nuget package based on platform.

Any thoughts? I am wondering if utilpack might still be adding value in the above proposition, in its ability to resolve dependencies accross platform, for anything that we might need to bundle inside the gitversiontask nuget package (newtonsoft.json ?) I.e not restored via nuget.

[stazz]

I understand the need to keep as little platform-specific code as possible. It appeals to me as well, and was the motivation behind UtilPack task factory itself.

Those 4 points you mention are definetly a step in a right direction in the sense of minimizing platform-specific code and still preserving the robustness of task for future, possibly binary-incompatible NuGet releases. The most hard thing would be probably point 2, because to even get stuff like NuGet package directory, and dedicated directory for specific package + version within the NuGet package directory requires using NuGet APIs. I could hardcode the logic without using NuGet-dependant code, but that might once again break in some future release. But it's definetly doable, just need to make it as parametrizable as possible :)

[stazz]

I've been thinking about this, and the recently opened issue ( #1554 ) has poked at those thoughts.

I think @dazinator has a viable approach idea. The value of the UtilPack in that scenario would indeed be that you could build for one target (e.g. netstandard1.3) and the UtilPack would take care of running everything for you. The only thing to settle in that case would be how to convey output/input parameters, as then they would collapse into the string array and a single integer (+ whatever files may be read/written).

In a technical view, this would cause the following changes:

• Instead of UtilPack, the NuGetUtils.MSBuild.Exec task factory would be used - it's basically an improved, new version of UtilPack (now that the UtilPack Git repository has been decomposed into multiple other repositories)
• The NuGetUtils.MSBuild.Exec would be compiled as .exe for net46+ and entrypointed dll for netcoreapp1.1+
• If the task factory detects that the package assembly does not contain any MSBuild ITask implementations, it would check if the package assembly has entrypoint information in it
• If the entrypoint information is found, the task factory starts itself via System.Diagnostics.Process API, which is OK, as it has been compiled as executable
• Here the task factory needs to serialize task parameters into string array for the entrypoint
• The executable entrypoint for NuGetUtils.MSBuild.Exec is different than the task factory: it will receive the target package information and execute it directly.

This way another process will be spawned which can load the specific NuGet packages from same directory without being disturbed by the NuGet version changes in .NET Core SDK itself. The process would be still running via NuGetUtils.MSBuild.Exec own entrypoint, which will take care of loading all dependant assemblies from their locations in NuGet package folder etc.

What do you think about this? I think it's a nice way to be able to lock down the NuGet version even on .NET Core, and only kink to clean is the heuristics of how to pass task parameters to the target entrypoint via string array.

[dazinator]

Interesting idea. So to check if I understand correctly:

1. We'd switch to use new task factory.

2. We'd point the new task factory towards our package id containing an exe or entry point assembly (instead of an ITask)

3. The new task factory would not do any msbuild restore in this case? Or it would but we would make sure there are no dependencies? (I'm guessing it wont do anything involving nuget otherwise we will have same issue)

4. The new task factory will launch itself in a seperate process passing state via args or stdin etc and most likely listening to STDOUT. (I guess for control messages)

5. The newly launched task factory (in child process) will be the one that launches our packages entry point / exe (and will it handle all nuget restore or dep resolution logic? even for an exe?) It would need to pass input params in as the args array. It would need some way of dealing with setting of msbuild props from output values from the tool. One way this could be done is to listen to.STDOUT for an agreed control message format i.e "##[Param=Foo]" would cause the listening util pack parent process to set that msbuild prop named "Param" to the value Foo accordingly? (just one idea)

6. The parent task factory process sets msbuild properties to corresponding values.

Am I close?

[stazz]

You are actually pretty much there. I have some old code lying around somewhere for stuff like process listening for named semaphore for cancellation, etc. That allows for the child process to at least try graceful shutdown when the parent (msbuild) process is requesting cancellation.

About the msbuild dependencies: I guess you have two choices, since there will be some need to deduce what kind of input and output parameters are needed for task, when msbuild requests them from the task factory.

1. Continue to have dependency to msbuild, and have a class implementing ITask which exposes all input and output parameters as properties, in a standard msbuild way. This class itself would never be instantiated by the task factory, it would be used to gather parameter information via reflection. This is good option when you still want to use your library as a msbuild task in some scenarios where the ex-UtilPack task factory is not needed. The task factory would need to be passed extra parameter in its XML telling it to use the normal entrypoint instead of instantiating task class.
2. Remove the msbuild dependency completely, and add dependency for NuGetUtils.Lib.EntryPoint, which is extremely simple library containing only one attribute. You then use the attribute to mark the actual entrypoint of your assembly, and let the NuGetUtils.MSBuild.Exec handle the rest.

My personal preference would be option number 2, as it cuts down the size of your dependencies substantially, but I guess I am a bit biased in this situation. ;)

An example of how your code would look like in scenario 2:

// Let the NuGetUtils.MSBuild.Exec know the entrypoint for this assembly
[assembly:  NuGetUtils.Lib.EntryPoint.ConfiguredEntryPointAttribute(
  typeof(GitVersionTask),
  nameof(GitVersionTask.Execute)
  )] 

public static class GitVersionTask {

  // The input parameter is created from command-line args by NuGetUtils.MSBuild.Exec, using standard Microsoft.Extensions.CommandLine package utilities
  // And the command lines are passed to this process by task factory which started this process.
  public static GitVersionTaskOutput Execute(
    GitVersionTaskInput input,
    CancellationToken token // This is optional but is handy
  ) {
    if (input.Validate()) { // Extension method, see below
      // Do the stuff here. Notice that you can not use any MSBuild stuff here (logging etc)
    } else {
      throw new ArgumentException("Invalid arguments.");
    }
  }
  
  // This is like marking properties with [Required] msbuild attributes, but you need to write the actual code yourself.
  // On a plus side, you can write any kind of complex logic here.
  private static Boolean Validate( 
    this GitVersionTaskInput input
  ) {
    return !String.IsNullOrEmpty( input?.RequiredOption );
  }
}

public sealed class GitVersionTaskInput {
  // All properties must have both getter and setter, for Microsoft.Extensions.Configuration packages
  public String RequiredOption { get; set; } 
  public String OptionalOption { get; set; }
}

public sealed class GitVersionTaskOutput {
  // Setter optional here, I think (haven't tried yet)
  public String SomeProperty { get; set; }
}

You can already try this code out using the .NET Global Tool NuGetUtils.Tool.Exec. All except the output parameter functionality is already in the current release, and if you're familiar how Microsoft.Extensions.Configuration packages read configuration from command line into types, then you should have really small learning curve.

In any case, the NuGetUtils.MSBuild.Exec has learned to cache restore results to disk, so only the first restore will be really slow one, the rest after that (even in new msbuild process) should be fast. Unless, of course, someone clears the cache folder on disk. :)

[asbjornu]

Resolved by #1677.