Use 2.0.0-beta3 roslyn compilers #947
Conversation
|
I assume you tested that these compilers run with the framework specified in the runtime.config file? |
|
I have additional changes for consuming this in CoreFx. No changes to runtime.config were required, but please let me know if alternative testing is required. |
|
Perhaps we were already in this world but I assume this now means we aren't using the Roslyn library that are part of the SharedFX which means they are not cross-gen'ed. That will likely have a noticeable impact on our build times. Have you noticed any decrease? |
|
@weshaggard I believe we have always been in this world for csc.exe itself. This is because ToolRuntime folder has always had a csc.exe which is not crossgened, and this means that the core host will use that instead of using the one inside the sharedFx. The Fx dependencies of csc.exe though are in fact crossgened since we pick the ones inside the cli. |
|
In corefx, on windows, the default is still too use the shared compiler, you have to opt into using this Roslyn compiler via "UseSharedCompilation=false". On Linux, the default is to use this Roslyn compiler. My changes (for this and subsequently corefx) do not affect the current behavior. |
|
That's a shame. Can we file a follow-up work-item to get all our corefx builds using the same compiler? If we don't we are quickly going to hit issues when people try to use the new features and they will work on some builds but not others. |
|
@chcosta I thought that the shared compiler that we use by default in Windows is also comming from the roslyn package, or is that not the case? |
|
That is not the case, the default compiler used is from MSBuild "C:\Program Files (x86)\MSBuild\14.0\bin\csc.exe" |
|
I know that is what the window says, but it might be worth to launch process explorer and take a look at what is actually running, because when you are using the shared compiler server, the output still will say that is running csc.exe even though it is actually running VBCSCompiler.exe. So let's just make sure which one is actually running. |
|
Ok, putting all the pieces together. Our build claims it is running csc.exe from MSBuild. I've validated that what it was actually doing was then running vbcscompiler from Roslyn tools. It wasn't clear to me exactly what vbcscompiler did, my assumption was that it was just responsible for choosing csc and launching csc, but joperezr explained that it is the compiler itself. Sooo.... I think that joperezr's last comment is correct, and there is no issue here. I'll close the issue I created to align compiler use across builds with some additional info. |
|
Even with VBCSCompiler.exe are we sure it is using the compiler from the package? Can you point me at the properties we are setting to make that happen? |
|
I have a subsequent PR for corefx which updates the properties there. This PR deals with updating the version of the compiler in buildtools. I can put up the corefx PR as well, I was blocking on this PR for that.. |
|
LGTM. We need to update the compiler version to run our tools on a newer shared framework, so we should get this in soon. |
|
The CoreFx PR is dotnet/corefx#10928 |
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <PropertyGroup> | ||
| <RoslynVersion>2.0.0-beta3</RoslynVersion> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Thanks @weshaggard , I've filed an issue to track removing some more of the duplication related to the Roslyn versions. If there is no other feedback, I will submit today so that I can focus on consuming this in CoreFx and getting a clean Jenkins run there. |
|
The rest LGTM. Thanks for migrating more to BuildTools. |
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now.
* Initial change to allow build wrappers and runtest.py * Build xunit wrappers on unix The generated wrapper needs to target netcoreapp on unix. I had to exclude assets from the xunit package and introduce a dependency on the private corefx bits, to resolve a dependency conflict in which the generated wrapper was depending on an older System.Runtime.dll than the helper library. I also disabled binclash logging, because the wrapper build binplaces the helper library to the same location multiple times. I couldn't find a simple way to disable binclash logging for the wrapper build only, since that requires passing an empty switch to run.exe, and bash word splitting makes this nontrivial from build-test. * Correctly generate TestEnv xplat Note that this will still require changes to the test wrapper to actually source the TestEnv on unix * Build xunit wrappers using SDK * Target netcoreapp2.0 in xunit wrappers This way, the wrappers can build even if the 2.1 SDK isn't installed on the machine. * Restore to packages directory for xunit wrappers * Move common properties out to dir.common.props When building wrappers using the SDK, we need some basic properties (like the build os/arch/config, and the output directories) to be set. I factored out properties used by both the old test build and the new SDK-project test build. At first I tried using Directory.Build.props (which is automatically imported by the SDK), but our test build already imports SDK targets in various places, so this was resulting in duplicate imports. Instead, I used dir.common.props, and made the imports explicit. * Remove desktop-specific test wrapper csproj * Pass build os/arch/type and logsdir to msbuild from runtest.py * Remove xunit wrapper helper library from traversal build * Fix parameter passing in build-test.sh Use bash arrays to pass parameters for the build command. This makes it possible to pass arguments with spaces to build_Tests_internal. We use this to disable binclashlogging selectively (for the xunit wrapper build only). * Clean up factored .props files * Undo runtest.sh changes * Use latest xunit console runner everywhere * Remove extra StaticDependency on xunit.runner.console * Eliminate tests/src/dir.common.props, and rename dir.sdkbuild.props tests/src/dir.common.props was only used for the desktop-specific xunit wrapper helper library. There's no need for it any more, so its properties have been moved into tests/src/dir.props. dir.sdkbuild.props has been renamed to dir.common.props, since it contains properties used by SDK projects and buildtools projects. This change also re-enables the test build. * Reintroduce dir.sdkbuild.props as a place for SDK-only props With this, some properties shared by SDK projects can go in a global location. The TargetFramework is shared by all SDK projects in the test tree. This change also uses a property for the xunit package directory that contains the xunit.console.dll we copy to core_root. * Add xml namespace to dir.common.props This fixes a failure in the windows build. * Satisfy xunit analyzer * Satisfy xunit analyzer again * Use SDK msbuild to build wrappers On windows, the use of run.exe, config.json, and msbuild.cmd uses msbuild.exe on the path. This change will build wrappers using the local SDK via "dotnet msbuild", bypassing run.exe. Run.exe will go away entirely with the move from buildtools to arcade, so other build invocatios should follow suit. * Remove Microsoft.CSharp.Core.targets workaround UseBuildTools used to be true all the time. Now that we are building wrappers on core, UseBuildTools becomes false. However, the rest of the runtest.proj expects to build using buildtools, so we keep UseBuildTools true until we switch to arcade. The CSharpCoreTargetsPath was imported when running on core only. This used to happen only on unix, but now it also happens when building runtest.proj for the xunit wrappers on windows. On unix, this targets file was a symlink to itself to work around some buildtools logic that expected the file to exist. This workaround no longer appears necessary, and on windows, this was never used in the first place, so this change removes it. * Remove UseRoslynCompilers prop and unify roslyn import UseRoslynCompilers was introduced in buildtools by dotnet/buildtools#947, with different behaviors on windows/unix. It was removed by dotnet/buildtools#1974, so we can unify our roslyn imports now. * Don't copy xunit dlls to corefx test host The corefx tests run on specific versions of xunit dlls, defined in CoreFX.depproj. We want to use these versions in the test host, not those in CORE_ROOT, so exclude these from being copied to the test host directory. This fixes the failing corefx tests. * Don't pass run.exe arguments through build-test.cmd in test pipeline These arguments get passed along to the xunit wrapper build as unprocessed build args. They need to work for "dotnet msbuild" (used for the wrapper build) as well as for run.exe. * Fix parameter passing of priority arg in build-test.cmd UnprocessedBuildArgs should contain arguments in the format expected by msbuild, not by run.exe. * Fix parameter passing of unprocessed args in build-test.cmd The "--" syntax is used by run.exe to pass everything following to msbuild directly. It should not be a part of unprocessed args. * Pass TargetsWindowsArg to wrapper build in build-test.cmd Helix builds tests on windows and runs them on unix using the xunit wrappers. When cross-building the wrappers like this, TargetsWindows is set to false by the test build pipeline. This variable ensures that the wrapper uses correct directory separators when invoking the test .sh file. * Pass BuildTestsAgainstPackages arg to exclude unix tests Helix builds xunit wrappers on windows, and runs them on unix. The BuildTestsAgainstPackages should currently be set to true in the windows wrapper build to properly filter the .cmd files based on exclusions in issues.targets.
Windows
BuildTools defines what version of the Roslyn compiler it restores (to tools\Microsoft.NETCore.Compilers) via buildtools:src/Microsoft.DotNet.Build.Tasks/PackageFiles/init-tools.cmd. BuildTools then copies Microsoft.NETCore.Compilers to tools\net45\roslyn for use on Windows desktop.
Linux
Version of Roslyn used is determined by the BuildTools:src\Microsoft.DotNet.Build.Tasks\PackageFiles\tools-runtime\project.json file which gets restored on the host OS to the Tools directory (tools\csc.exe). Linux uses the Microsoft.Net.Compilers.netcore package (vs Microsoft.Net.Compilers on Windows).
/cc @weshaggard @wtgodbe