Support .NET Core SDK 3.1.200+ #2221

rainersigwald · 2020-04-03T21:18:06Z

Description

Support MSBuild 16.5+ by changing what gets loaded into the GitLoaderContext.

Related Issue

Fixes #2063.

Motivation and Context

MSBuild 16.5 changed (dotnet/msbuild#4916) how MSBuild loads task assemblies to make it more robust to assemblies with mismatched dependencies or different versions of a specific task.

This caused problems with GitVersion because it took an inadvertent dependency on having parts of itself loaded in the default AssemblyLoadContext.

This change modifies the GitLoaderContext to know about the task assembly in whatever context it was loaded in, and augments it to load the other assemblies shipped in the GitVersionTask package.

How Has This Been Tested?

I built a trivial sample project referencing GitVersionTask using 3.1.200 and 3.1.101 (controlled via global.json) to validate that this fixes the problem on new MSBuild/SDK and continues to work on old MSBuild/SDK.

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.

arturcic · 2020-04-05T16:47:15Z

@rainersigwald do you have any idea why this test fails? https://dev.azure.com/GitTools/GitVersion/_build/results?buildId=2140&view=logs&j=c7677f77-9c7a-5d06-5685-83acb5683895&t=0b646eb6-eea0-55ee-7aae-c865cd16d489&l=520

Forgind

I don't think I understand the root of the problem well enough to approve this. We can talk about it if you'd like, or I can approve it anyway, since I think I understand the problem, if not what caused it, and the test you did sounded reasonable.

Forgind · 2020-04-05T17:00:29Z

src/GitVersionTask.MsBuild/LibGit2Sharp/GitLoaderContext.cs


        protected override Assembly Load(AssemblyName assemblyName)
        {
-            if (assemblies.Contains(assemblyName.Name))
+            if (assemblyName.Name == entryPointAssembly.GetName().Name)


Suggested change

if (assemblyName.Name == entryPointAssembly.GetName().Name)

if (assemblyName.Name.Equals(entryPointAssembly.GetName().Name))

Also, shouldn't we take into account OS to know if we can be case insensitive?

Forgind · 2020-04-05T17:07:50Z

src/GitVersionTask.MsBuild/LibGit2Sharp/GitLoaderContext.cs

                return LoadFromAssemblyPath(path);
            }

-            return Default.LoadFromAssemblyName(assemblyName);
+            return null;


It presumably would be messy to fully turn on nullable, since it would require modifying more code, but it would be nice to at least temporarily turn on nullable here, since you might return a null value. Then you could make sure every code path can handle null.

rainersigwald · 2020-04-06T16:14:32Z

@arturcic No, and for some reason VS didn't run it on my machine. When I select it explicitly I see it failing there. Looking.

arturcic · 2020-04-06T16:28:51Z

I'm using Rider and the net472 test are succeeding, only the 3.1 tests are failing.

rainersigwald · 2020-04-06T16:51:12Z

@arturcic Figured it out. Details in the commit message, but my change to allow loading more types from the task's folder in the GitLoaderContext meant that it loaded Microsoft.Build.Framework there only in the unit test, causing that failure (on .NET Core runtimes).

arturcic · 2020-04-06T16:53:33Z

Thanks a lot @rainersigwald, waiting for the build to finish and if green will merge it. Glad someone from the msbuild team helped us fixing this

This allows the runtime to handle attempting to load the file in another ALC, rather than forcing it to attempt to load it in the Default context.

When the entry-point task is loaded in an AssemblyLoadContext (as it is in MSBuild 16.5+ on .NET Core), the default ALC cannot load assemblies from next to the task location, so assemblies that are redistributed with the GitVersionTask package aren't automatically found. Extend the task's private ALC to support loading any assembly from the task's folder.

When MSBuild invokes a task from GitVersionTask.MsBuild, it creates an AssemblyLoadContext and loads GitVersionTask into it, then tries to load the type GitVersion.MSBuildTask.GitVersionTasks. The resulting type is then reflected over and a delegate is constructed to call the method inside the loaded type. However, if the task is loaded in its own AssemblyLoadContext, as it is in MSBuild 16.5+ on .NET Core, the types do not match between: 1. The WriteVersionInfoToBuildLog loaded in the task ALC (used in the cast), and 2. The WriteVersionInfoToBuildLog loaded in the GitLoaderContext (returned by reflection). This throws a runtime error. It worked in earlier versions of MSBuild because GitVersionTask.MsBuild was loaded in the default ALC by the MSBuild engine and found in the default ALC by the GetType() call. Change GitLoaderContext so that it returns the currently-loaded entry-point assembly when trying to load that assembly's identity. This works in older MSBuild, because it'll return the default ALC's instance of the type, and in newer MSBuild, because it'll return the task ALC's instance--in both cases accurately reflecting the task's view of the world.

This reverts commit 3f5392e, but leaves the SDK at 3.1.201.

Unit tests were failing because they load MSBuild assemblies from the test output directory. Normally they're not available there, so GitLoaderContext doesn't load them. But in tests, they were there, and being loaded in both ALC.Default (from the main part of the test) and a GitLoaderContext created a type mismatch ``` MissingMethodException: Method not found: 'Microsoft.Build.Framework.ITaskItem[] GitVersion.MSBuildTask.Tasks.UpdateAssemblyInfo.get_CompileFiles()'. at GitVersion.MSBuildTask.GitVersionTaskExecutor.UpdateAssemblyInfo(UpdateAssemblyInfo task) at GitVersion.MSBuildTask.GitVersionTasks.<>c__DisplayClass1_0.<UpdateAssemblyInfo>b__0(IGitVersionTaskExecutor executor) in D:\a\1\s\src\GitVersionTask\GitVersionTasks.cs:line 15 at GitVersion.MSBuildTask.GitVersionTasks.ExecuteGitVersionTask[T](T task, Action`1 action) in D:\a\1\s\src\GitVersionTask\GitVersionTasks.cs:line 30 ``` That was happening because the loaded `UpdateAssemblyInfo.CompileFiles` was of type (`Microsoft.Build.Framework.ITaskItem[]` in the `GitLoaderContext`) while it was looking for (`Microsoft.Build.Framework.ITaskItem[]` in `AssemblyLoadContext.Default`).

arturcic · 2020-04-06T16:56:22Z

Btw, @rainersigwald being you here, can you suggest some resources/links to github repos I can look at for integration test for msbuild tasks? I want to create some integration tests that will run the msbuild as process (.net core and full framework)

arturcic · 2020-04-06T17:20:56Z

@rainersigwald thank you so much for your contributions 👍

SeppPenner · 2020-04-06T17:28:22Z

This looks good. Now we just need a new version :)

arturcic · 2020-04-06T17:31:10Z

the new version will be 5.3.0, but I still have to finish some stuff. Hopefully this week we can release it

rainersigwald · 2020-04-06T17:33:24Z

can you suggest some resources/links to github repos I can look at for integration test for msbuild tasks? I want to create some integration tests that will run the msbuild as process (.net core and full framework)

We don't have any super easy way to do this, unfortunately. https://github.com/jeffkl/MSBuildProjectCreator can help.

We've talked about authoring some packages to make it easier to write a task (and test it), but it hasn't gotten to the top of the to-do list. dotnet/msbuild#3569 is the best work item I could find.

arturcic · 2020-04-06T17:34:12Z

Thank you, I'll check them

SeppPenner · 2020-04-06T17:45:26Z

the new version will be 5.3.0, but I still have to finish some stuff. Hopefully this week we can release it

That would be awesome 👍

admin-simeon · 2020-04-14T20:01:03Z

any update?

davidhjones · 2020-04-15T17:21:37Z

Update? Running into this issue trying to get a new build setup on sdk 3.1.201

arturcic · 2020-04-15T17:23:14Z

Sorry, busy on other projects these days, hope to get back this week

bonesoul · 2020-04-25T00:13:56Z

when we should expect a nuget beta release containing this fix?

rainersigwald mentioned this pull request Apr 3, 2020

the combination of VS 2019 Version 16.5.0 Preview 2.0 and .NET Core 3.1.200-preview-014883 fails dotnet build #2063

Closed

Forgind reviewed Apr 5, 2020

View reviewed changes

rainersigwald and others added 6 commits April 6, 2020 11:55

Return null from AssemblyLoadContext on unknown

b49cd49

This allows the runtime to handle attempting to load the file in another ALC, rather than forcing it to attempt to load it in the Default context.

Revert MSBUILDSINGLELOADCONTEXT

08c0b10

This reverts commit 3f5392e, but leaves the SDK at 3.1.201.

fixed some unit tests

1062bfc

rainersigwald force-pushed the assemblyloadcontext branch from 12bed98 to eeee6a0 Compare April 6, 2020 16:55

arturcic approved these changes Apr 6, 2020

View reviewed changes

arturcic merged commit 8923b87 into GitTools:master Apr 6, 2020

rainersigwald deleted the assemblyloadcontext branch April 6, 2020 17:50

ThomasPiskol mentioned this pull request Feb 28, 2021

[Bug] GitVersion.MsBuild does not set Azure DevOps build number #2552

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support .NET Core SDK 3.1.200+ #2221

Support .NET Core SDK 3.1.200+ #2221

rainersigwald commented Apr 3, 2020

arturcic commented Apr 5, 2020

Forgind left a comment

Forgind Apr 5, 2020

Forgind Apr 5, 2020

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

arturcic commented Apr 6, 2020

arturcic commented Apr 6, 2020

SeppPenner commented Apr 6, 2020

arturcic commented Apr 6, 2020

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

SeppPenner commented Apr 6, 2020

admin-simeon commented Apr 14, 2020

davidhjones commented Apr 15, 2020 •

edited

Loading

arturcic commented Apr 15, 2020

bonesoul commented Apr 25, 2020

	if (assemblyName.Name == entryPointAssembly.GetName().Name)
	if (assemblyName.Name.Equals(entryPointAssembly.GetName().Name))

Support .NET Core SDK 3.1.200+ #2221

Support .NET Core SDK 3.1.200+ #2221

Conversation

rainersigwald commented Apr 3, 2020

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Checklist:

arturcic commented Apr 5, 2020

Forgind left a comment

Choose a reason for hiding this comment

Forgind Apr 5, 2020

Choose a reason for hiding this comment

Forgind Apr 5, 2020

Choose a reason for hiding this comment

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

arturcic commented Apr 6, 2020

arturcic commented Apr 6, 2020

SeppPenner commented Apr 6, 2020

arturcic commented Apr 6, 2020

rainersigwald commented Apr 6, 2020

arturcic commented Apr 6, 2020

SeppPenner commented Apr 6, 2020

admin-simeon commented Apr 14, 2020

davidhjones commented Apr 15, 2020 • edited Loading

arturcic commented Apr 15, 2020

bonesoul commented Apr 25, 2020

davidhjones commented Apr 15, 2020 •

edited

Loading