net4x assets are not built and they are needed by VS code

[nguerrera]

There are net4x-specific assets in the closed build that are not produced in the source build. For standard dotnet usage, these assets are never used, but they are needed if msbuild is running on Mono or .NET Framework and pointed at the targets in the .NET Core SDK folder. VS Code's C# support does builds using Mono this way and things are broken without the net46 assets. VS for Mac has the same requirement. (Edit: So does Visual Studio.)

See dotnet/vscode-csharp#1744

We need to teach source build how to build these assets. In the closed build of dotnet/sdk, the assets are built on Windows and packaged on all platforms.

Major complication: targeting net4x using .NET Core msbuild on non-Windows is not yet officially supported. We can workaround it in various ways, or better yet implement it as a true supported feature, but it will require access to full framework reference assemblies during the build. And that probably means we need to build full framework reference assemblies from source...

Another path forward would be to make all of the task dlls (nuget too, not just dotnet/sdk) target .NETStandard and stop multi-targeting them to net4x and netcoreapp. However, it is currently next to impossible to make .NETStandard msbuild tasks. See dotnet/msbuild#1309 for some of the issues with it.

cc @DustinCampbell @Petermarcu @shawnro @dleeapho @karajas @leecow

[nguerrera]

cc @omajid

[omajid]

See OmniSharp/omnisharp-roslyn#919 for some related discussion.

Edit. Nope, that is a different issue. Thanks for correcting me.

[DustinCampbell]

@omajid: That's only tangentially related. In the case described by that issue, the .NET Core SDK being used was not built from source.

[nguerrera]

Adding @AndyGerlicher re: path forward to .NETStandard-based build tasks.

[jaredpar]

However, it is currently next to impossible to make .NETStandard msbuild tasks. See dotnet/msbuild#1309 for some of the issues with it.

Definitely. When we begin looking into this though we need to consider both the problems with a general task and a tool task. Tool tasks are doubly difficult to make NetStandard when they wrap a managed tool. There is no feasible way to implement ExecuteTool in that setup.

Consider the C# compiler task. How are we to implement ExecuteTool in a portable way? There are at least 3 different activation models we need to consider:

• Desktop: just run csc
• CoreClr: dotnet exec csc
• Mono: mono csc

[weshaggard]

All great points. While I would love to make the tasks build against .NET Standard there will definitely be some challenges.

In addition to this dependency on .NET Framework corefx also has a dependency on the targeting pack in order to generate the compat shims for .NET Core. We could fix that dependency by just generating the list of type-forwards manually and committing them to the repo.

As for building the .NET Framework reference assemblies from source we do have a flavor of that in the standard repo (https://github.com/dotnet/standard/tree/master/platforms/net461). It isn't a complete set but it gives us a model and the possibility to produce the reference assemblies from source.

[DustinCampbell]

It sounds to me like .NET Standard tasks are a bit into the future yet -- at least for tool tasks as Jared mentions. Without some sort of NetStandard Exe (which seems like a good feature to pursue at some point), I can't see how we could make tool tasks work without some sort of nasty shenanigans.

It seems like having net46 assets build from source is something we need.

[weshaggard]

I can't see how we could make tool tasks work without some sort of nasty shenanigans.

That isn't a .NET Standard issue exactly that is more of a hosting issue for the platform you are running on (i.e. .NET Core/mono/etc). We definitely need to figure out a strategy to that problem regardless of whether or not these ToolTasks cross-compile for .NET Framework. In the case of ToolTask I think the only thing we can really do is use the host that msbuild is running on to execute these custom tools.

[DustinCampbell]

Agreed. And if those custom tools were some sort of NetStandard Exe, there would be a much better chance that the current runtime host could run them.

Regardless, being able to build net4x cross platform is a valid scenario that will require build from source for .NET Framework reference assemblies. Going down the road of focusing on MSBuild NetStandard tasks would only solve this particular issue, and not the general customer scenario.

[omajid]

To clarify, this affects everyone using omnisharp-roslyn, not just VSCode. Emacs and Eclipse would be affected too.

Surprisingly, vim which uses omnisharp-server is immune ;)

[DustinCampbell]

I don't believe omnisharp-server supports .NET Core 2.0 though. Does it even support .NET Core at all?

[omajid]

I don't believe omnisharp-server supports .NET Core 2.0 though. Does it even support .NET Core at all?

Honestly, no idea. But I have a co-worker who uses this and has worked on (simple) C# projects that are targetting .NET Core 2.0

[DustinCampbell]

FWIW, the last commit into omnisharp-server was 2.5 years ago and it requires Mono to be installed in order to run. If .NET Core 2.0 does work with it (I haven't verified), it'd be because Mono supports it in their targets.

[tmds]

How feasible is it to make omnisharp run on .NET Core instead of mono?
I assume we could then use the assets that are already built by source-build?

[tmds]

How feasible is it to make omnisharp run on .NET Core instead of mono?
I assume we could then use the assets that are already built by source-build?

omnisharp-roslyn is an important part of the open xplat .NET Core ecosystem, it would be really nice if it would run on .NET Core.

[DustinCampbell]

@tmds: omnisharp-roslyn used to be run on CoreCLR but there were too many issues with handling non .NET Core projects. Because omnisharp-roslyn needs to support such projects (e.g. .NET Framework, Unity, Xamarin, etc.), it can't run on CoreCLR.

[tmds]

@tmds: omnisharp-roslyn used to be run on CoreCLR but there were too many issues with handling non .NET Core projects. Because omnisharp-roslyn needs to support such projects (e.g. .NET Framework, Unity, Xamarin, etc.), it can't run on CoreCLR.

I understand the reasoning. I also read: OmniSharp/omnisharp-roslyn#915.
This has been done pre-netstandard2.0/.NET Core 2.0.
As you put in the PR:

Note: This change does not move our libraries over to netstandard2.0. I think that's possibly the next thing, but it's not needed yet.

I think moving to netstandard2.0 would be very meaningful. It will make it possible to use OmniSharp from .NET Core.
Perhaps this is an area where we (Red Hat) can contribute to OmniSharp.

[DustinCampbell]

@tmds: Moving OmniSharp's libraries to netstandard2.0 is orthogonal. The issue is that MSBuild is somewhat special in that it runs user code (in the form of tasks) in order to process projects. In order for users to be able to successfully load legacy projects (or even non-legacy projects like Unity or Xamarin), MSBuild (which runs in OmniSharp's process) must be able to run those tasks, which are not designed to run on CoreCLR.

[tmds]

@tmds: Moving OmniSharp's libraries to netstandard2.0 is orthogonal. The issue is that MSBuild is somewhat special in that it runs user code (in the form of tasks) in order to process projects. In order for users to be able to successfully load legacy projects (or even non-legacy projects like Unity or Xamarin), MSBuild (which runs in OmniSharp's process) must be able to run those tasks, which are not designed to run on CoreCLR.

I'm wondering if it is feasible to have a meaningful part of it target netstandard2.0 so that part can be used in .NET Core only use-cases.

[DustinCampbell]

I'm wondering if it is feasible to have a meaningful part of it target netstandard2.0 so that part can be used in .NET Core only use-cases.

It's certainly possible, but I don't believe it's a good long-term solution. When users open mixed solutions with .NET Core and other project types, it's a bad experience when half of the projects fail to provide IntelliSense.

[tmds]

It's certainly possible, but I don't believe it's a good long-term solution. When users open mixed solutions with .NET Core and other project types, it's a bad experience when half of the projects fail to provide IntelliSense.

I agree, e.g. vscode should behave the same on different platforms.
A question that is more on-topic: the source-build sdk contains a netcoreapp1.0 version of the assembly omnisharp is trying to use. Could omnisharp use that one (if it doesn't find the net46 assembly)? Or is this netcoreapp1.0 assembly definitely incompatible with mono?

[omajid]

Ping! Just trying to restart the conversation.

[omajid]

It's certainly possible, but I don't believe it's a good long-term solution. When users open mixed solutions with .NET Core and other project types, it's a bad experience when half of the projects fail to provide IntelliSense.

That is true, but right now the situation is worse for everyone who builds .NET Core from sources. They get no IntelliSense, even in pure .NET Core projects :(

[DustinCampbell]

FWIW, if you're willing to do that, you should be able to install Mono and build OmniSharp itself. Then, it's just a matter of setting the "omnisharp.path" and "omnisharp.useMono" options is VS Code.

[tmds]

The early didn't have the symlink patch yet. Unfortunately adding the link leads to a different error:

[fail]: OmniSharp.MSBuild.ProjectLoader
        The "CheckForDuplicateItems" task failed unexpectedly.
System.TypeLoadException: Could not resolve type with token 0100007d (from typeref, class/assembly System.Globalization.CultureInfo, System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a)
  at Microsoft.NET.Build.Tasks.TaskBase.Execute () [0x00000] in <f36329163db64545bff8b6316f824029>:0 
  at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute () [0x00023] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
  at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__26.MoveNext () [0x00212] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
[fail]: OmniSharp.MSBuild.ProjectLoader
        The "CheckForDuplicateItems" task failed unexpectedly.
System.TypeLoadException: Could not resolve type with token 0100007d (from typeref, class/assembly System.Globalization.CultureInfo, System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a)
  at Microsoft.NET.Build.Tasks.TaskBase.Execute () [0x00000] in <f36329163db64545bff8b6316f824029>:0 
  at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute () [0x00023] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
  at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__26.MoveNext () [0x00212] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
[fail]: OmniSharp.MSBuild.ProjectLoader
        The "CheckForDuplicateItems" task failed unexpectedly.
System.TypeLoadException: Could not resolve type with token 0100007d (from typeref, class/assembly System.Globalization.CultureInfo, System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a)
  at Microsoft.NET.Build.Tasks.TaskBase.Execute () [0x00000] in <f36329163db64545bff8b6316f824029>:0 
  at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute () [0x00023] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
  at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__26.MoveNext () [0x00212] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
[fail]: OmniSharp.MSBuild.ProjectLoader
        The "GetAssemblyVersion" task failed unexpectedly.
System.IO.FileNotFoundException: Could not load file or assembly 'NuGet.Versioning, Version=4.7.0.1, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies.
File name: 'NuGet.Versioning, Version=4.7.0.1, Culture=neutral, PublicKeyToken=31bf3856ad364e35'
  at Microsoft.NET.Build.Tasks.TaskBase.Execute () [0x00000] in <f36329163db64545bff8b6316f824029>:0 
  at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute () [0x00023] in <9ba305957e954df9b0ed46d97ba8f5be>:0 
  at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__26.MoveNext () [0x00212] in <9ba305957e954df9b0ed46d97ba8f5be>:0 

[tmds]

I guess what we have now under Sdks/Microsoft.NET.Sdk/tools depends on things which aren't provided by the (mono) framework that comes with omnisharp.
With 2.0, the tools had a netcoreapp1.0 folder and for 2.1, it is a netcoreapp2.0 folder. Maybe we are relying on a larger API surface which isn't completely supported by mono.

[tmds]

Maybe we are relying on a larger API surface which isn't completely supported by mono.

I'm not sure this is the problem. Looking in Microsoft.NET.Build.Tasks, there isn't much 2.0 specific stuff.

Some things get excluded when building Microsoft.NET.Build.Tasks: https://github.com/dotnet/sdk/blob/c4520055d2b3e21667a32471561b5a51f1b31066/src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj#L50-L54

  <!-- These are loaded from the CLI's copy on .NET Core, we don't need to duplicate them on disk -->
  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.0'">
    <PackageReference Update="Microsoft.Extensions.DependencyModel" ExcludeAssets="Runtime" />
    <PackageReference Update="NuGet.ProjectModel" ExcludeAssets="Runtime" />
  </ItemGroup>

Maybe if we're including these things in source-build, omnisharp will find these missing bits?

[DustinCampbell]

@tmds : We have a new beta release of C# for VS Code that may help, though I'm unsure. You can download the VSIX from https://github.com/OmniSharp/omnisharp-vscode/releases/tag/v1.15.0-beta6 and follow the steps at the Installing Beta Releases wiki page to give it a shot.

[tmds]

@DustinCampbell using v1.15.0-beta6 I get a different error:

[fail]: OmniSharp.MSBuild.ProjectLoader
        The "ResolvePackageAssets" task could not be instantiated from "/home/tmds/Downloads/dotnet-omajid/sdk/2.1.300-preview2-006908/Sdks/Microsoft.NET.Sdk/targets/../tools/net46/Microsoft.NET.Build.Tasks.dll". Could not load type of field 'Microsoft.NET.Build.Tasks.ResolvePackageAssets:TextEncoding' (26) due to: Could not resolve type with token 01000066 (from typeref, class/assembly System.Text.Encoding, System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a) assembly:System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Text.Encoding member:(null) signature:<none>

I don't know if this means it got further or if it is failing earlier.

[DustinCampbell]

I'm not sure either. 😞

[nguerrera]

I think the reference assembly issue has been resolved for source-build and that we should be able to just fix this. cc @dagood @weshaggard is there a full framework reference assembly package that we can use in source build now? I'd be happy to tweak the dotnet/sdk build to use that and remove the '$(OS)' == 'Windows_NT' from our projects.

It was always pure luck that the symlink hack worked and I don't think we should invest in debugging that, but rather aim to fix this properly.

[tmds]

@nguerrera it would be the best if we can build the same net46 folder like Microsoft builds!

Including the bits that are not in netcoreapp2.0 Microsoft.NET.Build.Tasks (#125 (comment)) gives the same error on C# v1.14 as it does without patching on v1.15.0-beta6 (#125 (comment)):

The "ResolvePackageAssets" task could not be instantiated from "/home/tmds/repos/source-build/src/cli/bin/2/linux-x64/dotnet/sdk/2.1.300-preview2-006908/Sdks/Microsoft.NET.Sdk/targets/../tools/net46/Microsoft.NET.Build.Tasks.dll". Could not load type of field 'Microsoft.NET.Build.Tasks.ResolvePackageAssets:TextEncoding' (26) due to: Could not resolve type with token 01000066 (from typeref, class/assembly System.Text.Encoding, System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a) assembly:System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Text.Encoding member:

I guess this means mono is unable to locate 'System.Text.Encoding'. Why is that? Does the netcoreapp dll say this type lives in a different place than the net46 dll? Or what is causing this?

[nguerrera]

I think it simply means that the .NET Core 2.0 version of System.Text.Encoding (4.2.0.0) is not supported by the version of Mono being used. I believe that version is greater than any version supported on any version of .NET Framework or .NET Standard.

Generally speaking, if you target .NET Core, you should not expect to run on Mono. Like I said, the hack working was pure luck.

A better (still unsupported, but I would at least expect it to work) workaround would be to grab the net46 folder from another distribution of the SDK and copy it over to the source-built one.

But again, let's just fix this. :)

[tmds]

cc @dagood @weshaggard is there a full framework reference assembly package that we can use in source build now?

@nguerrera @dagood @weshaggard I guess this is using dotnet/designs#33?

The PR includes this line:

• We need to determine what license to use for these packages.

Maybe we have some requirement if we want to use this from source-build.

[tmds]

@nguerrera @dagood @weshaggard can you please chime in to see what is feasible for 2.1?

[weshaggard]

@tmds honestly I'm not sure what the ask is any more. If the ask is around nuget packages for .NET Framework targeting packs then as you point out we have a design dotnet/designs#33 for that but those aren't going to be done in the .NET Core 2.1 timeframe. We do have existing nuget packages we use for our builds that could be used as a stop gap.

However if the issue is about making OmniSharp/VS code work on .NET Core then that should be a separate issue and probably tracked in the vscode repo not here.

[tmds]

We want to build the net46 version of Microsoft.NET.Build.Tasks that is used by vscode/omnisharp. Using the existing nuget packages is fine.

[nguerrera]

However if the issue is about making OmniSharp/VS code work on .NET Core then that should be a separate issue and probably tracked in the vscode repo not here.

This is the right place to track it. There are parts of the dotnet/sdk build that are turned off in source build and so dotnet/sdk from source build is not able to run on Mono. This is not something VS Code can fix, we have to fix the source build.

We do have existing nuget packages we use for our builds that could be used as a stop gap.

Do you reference these during source build?

[weshaggard]

This is the right place to track it. There are parts of the dotnet/sdk build that are turned off in source build and so dotnet/sdk from source build is not able to run on Mono. This is not something VS Code can fix, we have to fix the source build.

OK but I think that at least deserves its own issue.

Do you reference these during source build?

Yes rolsyn-tools which is now part of source-build references it and so it is pulled into our prebuilt packages. see https://github.com/dotnet/roslyn-tools/blob/master/sdks/RepoToolset/tools/ProjectDefaults.props#L136.

[weshaggard]

OK but I think that at least deserves its own issue.

That is in my mind I thought this issue was tracking the targeting pack not necessarily the SDK changes.

[nguerrera]

Yes rolsyn-tools which is now part of source-build references it and so it is pulled into our prebuilt packages. see https://github.com/dotnet/roslyn-tools/blob/master/sdks/RepoToolset/tools/ProjectDefaults.props#L136.

Awesome. We can do that in SDK to fix this then.

[nguerrera]

I filed dotnet/sdk#2249. We will not be able to get this fixed in the first 2.1 release (SDK 2.1.300) because that is locked down, but we can fix it for 2.1.400+.

[tmds]

@nguerrera thanks, I hope 2.1.400 comes soon then.
An IDE without a language server is like a pub without beer.

[nguerrera]

Closing this issue because #125 (comment) confirms that source-build itself is no longer blocking components within it from having net4x assets. dotnet/sdk#2249 tracks the actual fix that is now unblocked.