Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@ausbin
Copy link
Contributor

@ausbin ausbin commented Jun 15, 2022

…on conflicts (#566)"

Without this PR, some tests such as the LoadOutdatedQSharpProject test in ProjectLoaderTests were failing as follows:

 [Warning]: MSBuild warning in C:\Program Files\dotnet\sdk\6.0.301\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.EolTargetFrameworks.targets(28,5): The target framework 'netcoreapp2.0' is out of support and will not receive security updates in the future. Please refer to https://aka.ms/dotnet-core-support for more information about the support policy.
 [Warning]: MSBuild warning in C:\Program Files\dotnet\sdk\6.0.301\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.EolTargetFrameworks.targets(28,5): The target framework 'netcoreapp2.0' is out of support and will not receive security updates in the future. Please refer to https://aka.ms/dotnet-core-support for more information about the support policy.
 [Error]: MSBuild error in C:\Program Files\dotnet\sdk\6.0.301\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(267,5): The "ResolvePackageAssets" task failed unexpectedly.
 System.IO.FileLoadException: Could not load file or assembly 'NuGet.ProjectModel, Version=6.2.1.2, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. Could not find or load a specific file. (0x80131621)
 File name: 'NuGet.ProjectModel, Version=6.2.1.2, Culture=neutral, PublicKeyToken=31bf3856ad364e35'
  ---> System.IO.FileLoadException: Could not load file or assembly 'NuGet.ProjectModel, Version=6.2.1.2, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.
    at System.Runtime.Loader.AssemblyLoadContext.LoadFromPath(IntPtr ptrNativeAssemblyLoadContext, String ilPath, String niPath, ObjectHandleOnStack retAssembly)
    at System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyPath(String assemblyPath)
    at Microsoft.Build.Shared.MSBuildLoadContext.Load(AssemblyName assemblyName)
    at System.Runtime.Loader.AssemblyLoadContext.ResolveUsingLoad(AssemblyName assemblyName)
    at System.Runtime.Loader.AssemblyLoadContext.Resolve(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader.CreateReaderFromMemory(ResolvePackageAssets task, Byte[] settingsHash)
    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader..ctor(ResolvePackageAssets task)
    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ReadItemGroups()
    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ExecuteCore()
    at Microsoft.NET.Build.Tasks.TaskBase.Execute()
    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)
 [Error]: Failed to resolve assembly references for project 'C:\Users\t-aadams\Documents\qsharp-compiler\src\QsCompiler\Tests.LanguageServer\bin\Debug\net6.0\TestProjects\test9\test9.csproj'.

This reverts commit 5fc3bab, aka PR #566, which looks like it was intended to resolve this issue in the past. But from printfs I added, I don't think that additional code was doing anything in this newer version of .NET to locate NuGet assemblies required by msbuild. I don't know why, but this PR will remove that code as it no longer seems necessary to me.

To buy some time for an msbuild god's help, we can upgrade our version of NuGet.ProjectModel. It has already been referenced in the language server project file since 0b3d4b3 anyway.

Testing

  • Ran language server locally via VSCode
  • Ran all QsCompiler unit tests and they passed.

return null;
};
}
static ProjectLoaderTests() =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I am not sure this change here will always work correctly - I believe the locator throws if registering defaults is called multiple times or too late. I suggest to keep it as an AssemblyInitialize method, though I may be wrong about the need/benefits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into some cases where AssemblyInitialize was not being respected correctly; on the IQ# kernel, isolated the call from unit testing harness to RegisterDefaults in a Lazy<VisualStudioInstance> property on a static class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed up a new version. Is that what you meant, @cgranade?

@ausbin ausbin force-pushed the aadams/update-nuget-projectmodel branch from aa2f725 to 3fe02c6 Compare June 15, 2022 22:35
@bettinaheim bettinaheim marked this pull request as ready for review June 16, 2022 02:25
@bettinaheim bettinaheim merged commit 5990f08 into main Jun 16, 2022
@ausbin ausbin deleted the aadams/update-nuget-projectmodel branch June 16, 2022 02:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants