Updating UtilPack.NuGet.MSBuild to NuGetUtils.MSBuild.Exec

[stazz]

Hi,

As discussed in #1554 and #1537 , there is a need to change UtilPack's way of executing everything in MSBuild process, as that causes big problems with NuGet versioning (binary-incompatible changes even on minor version change, and the dotnet build command lock NuGet version to specific one).

This change contains various modifications in GitVersionTask project, which makes it:

• Compatible with NuGetUtils.MSBuild.Exec, and
• Independant of Microsoft.Build.Utilities.Core package.

The GitVersionTask is now invoked via separate process by task factory of NuGetUtils.MSBuild.Exec package.

I have run only very preliminary tests so far, but starting PR process already to see how automated tests fare as well.

[stazz]

FYI, I've found out that the CI build process generated faulty .(dll|exe).config files, and NuGetUtils.MSBuild.Exec currently doesn't run on .NET Desktop. I'm in a process of fixing it, I found out what I was missing and I just need to write some code to patch things up. I'll keep you posted.

[stazz]

For my part, the pull request is ready for review. I've tested with small test .csproj with both .NET Desktop and .NET Core, and I think Mono should be fine too, since AppDomains are no longer used anyway.

I'm not sure why the checks fail - based on messages, they seem quite random (SSL errors or npm failing).

[stazz]

Also, one thing I should mention: the NuGetUtils.MSBuild.Exec package has now the following minimum supported frameworks:

• .NET 4.6 for .NET Desktop, and
• .NET Core 2.1 for .NET Core.

These can be worked on, though, if the minimum requirements are gonna cause problems.

[asbjornu]

Regarding the upgrade of requirements to .NET 4.6 and .NET Core 2.1, I think it's fine. We already have breaking changes planned for 5.0.0, so I think it's a good fit.

[stazz]

@asbjornu Glad to hear the requirements upgrade is ok! Related to #1644 , yeah, the GitVersionTask no longer uses MSBuild logging at all. In some way, this is a bit of regression, but I'm sure it can be fixed by introducing concept of verbosity to input parameters of tasks, which can they propagate it to loggers.

Right now, I can't think of any quick fixes which could bring any major improvement to NuGetUtils or to this PR. @dazinator , just ping me if you need any help with the review, I think the changes are quite straightforward.

[asbjornu]

I see. Removing the integration with MSBuild might be a bummer for some users. What are your thoughts on this, @dazinator and @arturcic?

[stazz]

@asbjornu The detailed discussion about this is in issues #1554 and #1537 , but to summarize:

• When dotnet build command is invoked, it locks down the NuGet assembly versions (marking them Trusted Platform Assemblies) so that the process can not load any NuGet assemblies with different version than the ones shipped with current version of dotnet.
• The ex-UtilPack, now NuGetUtils, task factory which loads and executes GitVersionTask uses NuGet assemblies to deduce dependencies and load them on-the-fly from their package directories within NuGet repository. This way, GitVersionTask does not need to ship all dependencies within its package.
• NuGet assemblies often get binary-incompatible changes even in minor releases, causing the task factory to fail uglily when users run GitVersionTask using dotnet with binary-incompatible NuGet assemblies than the ones which task factory was compiled against
• One solution is to always re-release the task factory whenever NuGet gets new versions, but that is not scalable solution. So, to fix this properly, the task factory was refactored to handle all NuGet-related stuff (restoring, dependency deduction, actual execution) in separate processes which it invokes.
• As a direct consequence of this, the GitVersionTask no longer has access to IBuildEngine instance.

When I talked with @dazinator some months ago about this, he said that losing access to MSBuild stuff (ITask, IBuildEngine) was not a big issue. When I did code changes, I didn't notice anything MSBuild-dependant, other than logging.

If access to MSBuild-specific stuff is critical, I guess you would need to give up on using NuGetUtils task factory, and ship dependencies within the NuGet package, and handle native assembly loading logic. I guess the @dazinator and @arturcic know the best route to proceed here.

[asbjornu]

Thanks for the thorough explanation, @stazz. Personally, I have no problems with dropping the MSBuild integration, but I'm not sure I'm representative of GitVersion's full audience. Let's wait for @dazinator and @arturcic's input before we move ahead.

[dazinator]

Sorry I haven't had a chance to look at this yet, will try and pick up this weekend!

[asbjornu]

Now that #1644 is merged, I'm afraid we need you to rebase and resolve the conflicts of this PR, @stazz. To avoid the merge commits, could I ask you to please do the following?

git remote add upstream https://github.com/GitTools/GitVersion.git
git fetch upstream
git checkout stazz/NuGetUtilsUpdate
git rebase upstream/master
# Resolve conflicts, add and commit
git push --force

[asbjornu]

Seems like that rebase didn't quite work out. No worries, I'll fix it in #1645, which will resolve this when merged.

[asbjornu]

/rebase

[autorebase]

The rebase failed:

Merge conflict

To rebase manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/rebase stazz/NuGetUtilsUpdate
# Navigate to the new directory.
cd .worktrees/rebase
# Rebase and resolve the likely conflicts.
git rebase --interactive --autosquash master
# Push the new branch state to GitHub.
git push --force
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/rebase

[dazinator]

@arturcic I'm holding off the review for this as it looks like there are conflicts to resolve on the proj files still. Once this PR is in good shape again i'll happily review.

[dazinator]

Conflicts to resolve atm

[asbjornu]

@dazinator, I've resolved the conflicts and performed a fresh rebase in #1645. Can you please review that?