GH1540: Update to latest Roslyn #1645

patriksvensson · 2017-06-14T15:00:13Z

This PR will do the following:

Update all projects to target net462 instead of net45
Update Roslyn to the latest version
Remove old Roslyn (regular and experimental)
Remove the Mono scripting engine
Unify script engine between full framework and .net core
Make it build on CI again
Add loading assembly from stream for debugging purposes (see AssemblyLoader).
Verify that debugging works on full framework and .NET Core
Clean up the code a bit

bjorkstromm · 2017-06-14T15:03:48Z

build/paths.cake

@@ -26,7 +26,7 @@ public class BuildPaths
        var buildDir = context.Directory("./src/Cake/bin") + context.Directory(configuration);
        var artifactsDir = (DirectoryPath)(context.Directory("./artifacts") + context.Directory("v" + semVersion));
        var artifactsBinDir = artifactsDir.Combine("bin");
-        var artifactsBinNet45 = artifactsBinDir.Combine("net45");
+        var artifactsBinNet45 = artifactsBinDir.Combine("net462");


Also change variable name to artifactsBinNet462

devlead · 2017-06-26T06:26:47Z

@cake-build/cake-team I've removed some unused variables in path.cake and added so chocolatey/desktop CLR nuget content isn't hard coded any more. so we'll need to test so those still include all's that needed.

devlead · 2017-07-17T13:04:38Z

@cake-build/cake-team I've sorted the merge conflicts and rebased against latest develop.

gep13 · 2017-07-19T11:16:08Z

Relates to #1540

For some reason GitHub won't "see" that the changes has been made.

patriksvensson · 2017-07-23T23:21:39Z

@mholo65 Did you have a fix for the debugging that we could use?

bjorkstromm · 2017-07-24T06:14:43Z

@patriksvensson only the AppDomain.AssemblyResolve fix, but that feels wrong. IMO we need to investigate further and ask some help from Roslyn team. How is it working with CoreCLR debugging, same thing there?

patriksvensson · 2017-07-24T08:41:09Z

@mholo65 Not sure, and I won't have time to do anything with this PR until the weekend soonest.

bjorkstromm · 2017-07-24T08:42:18Z

@patriksvensson Ok! I'll see what I can do!

bjorkstromm · 2017-07-24T11:44:46Z

Seems to hit the FileNotFoundException when referencing addins and debugging. Will investigate further, trying to create a minimal repro and then contact Roslyn team if I can't figure it out myself.

Also, I seem to hit same exception when debugging with Cake.CoreCLR version 0.21.1. Could anyone of you verify this?

filipw · 2017-07-24T14:09:39Z

you might consider going straight for Roslyn 3 (nightly) and avoid hand-rolled debugging code altogether 🙂 dotnet/roslyn#16489

filipw · 2017-07-24T14:12:22Z

actually, I forgot - that made it into Roslyn 2.3.0 already

bjorkstromm · 2017-07-24T14:14:29Z

@filipw thanks for the heads up! @patriksvensson we should update to 2.3.x and check that out!

bjorkstromm · 2017-07-24T16:57:45Z

@filipw got any examples for using the API? Will have a look at it tonight!

gep13 · 2017-07-24T17:00:42Z

@mholo65 said...
Could anyone of you verify this?

As discussed on slack, i have been able to verify this is happening.

filipw · 2017-07-24T18:07:35Z

@mholo65 try here https://github.com/filipw/dotnet-script/blob/master/src/Dotnet.Script.Core/ScriptCompiler.cs#L53

bjorkstromm · 2017-07-24T21:35:10Z

src/Cake/Scripting/Roslyn/RoslynScriptEngine.cs

        {
            _options = options;
-            _nightlyFactory = nightlyFactory;
-            _stableFactory = stableFactory;
+            _loader = loader;
            _log = log;
        }

        public IScriptSession CreateSession(IScriptHost host, IDictionary<string, string> arguments)


While we are at it breaking stuff. Maybe we should remove arguments from the interface?

Agree! 👍

bjorkstromm · 2017-07-24T21:49:44Z

Roslyn now updated and debugging works. Unified the sessions also.

Thanks again @filipw for pointing us in the right direction!

filipw · 2017-07-24T21:52:18Z

great! 🎉

patriksvensson · 2017-07-24T23:07:01Z

@cake-build/cake-team OK! I think this PR is ready to be merged. Do you mind taking a look at it and also take it for a final spin if you have time.

gep13

Just one thing...

gep13 · 2017-07-25T06:37:40Z

src/Cake/Arguments/ArgumentParser.cs

@@ -166,16 +166,6 @@ private bool ParseOption(string name, string value, CakeOptions options)
                options.PerformDebug = ParseBooleanValue(value);
            }

-            if (name.Equals("mono", StringComparison.OrdinalIgnoreCase))


@patriksvensson @mholo65 I am not sure if these should be removed completely. I think we should deprecate these commands. i.e. leave them in just now, but have them only emit an error that the Mono scripting engine has been removed with an explanation as to why. Then in a future version, we can remove them completely. Thoughts?

Removing them won't break anything afaik.

patriksvensson · 2017-07-25T09:53:45Z

@Meir017 Yes, it's still used here: https://github.com/cake-build/cake/pull/1645/files#diff-3c3b3d790a1cf0ee07879f36796b4230R33

agc93

With the new warning in place, LGTM at least 👍

bjorkstromm

LGTM! Could we also start integration tests on this branch prior to merging?

devlead · 2017-07-26T06:55:50Z

@mholo65 triggered an integration test on Ubuntu with Bitrise now.

patriksvensson · 2017-07-28T14:08:50Z

@gep13 Not sure why, but your review doesn't seem to close although the changes has been addressed.

gep13 · 2017-07-28T14:14:59Z

@patriksvensson fixed

patriksvensson · 2017-07-28T14:47:00Z

@gep13 Thanks!
@devlead What was the result of the integration test?

devlead · 2017-07-28T15:34:05Z

@patriksvensson it didn't complete but don't think it's a PR issue will try to get them to rerun

devlead · 2017-07-29T00:13:58Z

build.cake

        VersionSuffix = parameters.Version.DotNetAsterix,
        Configuration = parameters.Configuration,
-        OutputDirectory = parameters.Paths.Directories.ArtifactsBinNet45,
+        OutputDirectory = parameters.Paths.Directories.ArtifactsBinFullFx,
        Verbosity = DotNetCoreVerbosity.Minimal


@patriksvensson Currently getting an error with copy files on Linux

/usr/share/dotnet/sdk/1.0.4/Microsoft.Common.CurrentVersion.targets(1111,5): error MSB3644: The reference assemblies for framework ".NETFramework,Version=v4.6.2" were not found. To resolve this, install the SDK or Targeting Pack for this framework version or retarget your application to a version of the framework for which you have the SDK or Targeting Pack installed. Note that assemblies will be resolved from the Global Assembly Cache (GAC) and will be used in place of reference assemblies. Therefore your assembly may not be correctly targeted for the framework you intend. [/cake/src/Cake/Cake.csproj] An error occurred when executing task 'Copy-Files'.

Adjusting copy files task like below seems to sort it

// .NET 4.6.2 DotNetCorePublish("./src/Cake", new DotNetCorePublishSettings { Framework = "net462", VersionSuffix = parameters.Version.DotNetAsterix, Configuration = parameters.Configuration, OutputDirectory = parameters.Paths.Directories.ArtifactsBinFullFx, - Verbosity = DotNetCoreVerbosity.Minimal, + MSBuildSettings = msBuildSettings }); // .NET Core DotNetCorePublish("./src/Cake", new DotNetCorePublishSettings { Framework = "netcoreapp1.0", Configuration = parameters.Configuration, OutputDirectory = parameters.Paths.Directories.ArtifactsBinNetCore, - Verbosity = DotNetCoreVerbosity.Minimal, + MSBuildSettings = msBuildSettings });

devlead

LGTM now, but think we should rebase and squash a few commits before the final review & merge

bjorkstromm · 2017-07-29T15:22:07Z

Squash is Ok as long as individual authors changes are kept intact. I.e. Should be squashed to 4 or 5 commits.

devlead · 2017-07-29T15:23:00Z

Squash is Ok as long as individual authors changes are kept intact. I.e. Should be squashed to 4 or 5 commits.

@mholo65 agreed 👍

* Updated projects to net462. * Cleaned up the project files. * Removed old versions of Roslyn. * Removed Mono scripting engine. * Added Roslyn script engine for full fx.

* Removed unused build script variables. * Cleaned up the build script.

* Update Roslyn to 2.3.1. * Use standardized way of emitting debug symbols for debug build. * Unify script sessions.

* Removed Mono.CSharp from license information. * Removed Roslyn NuGet source constant. * Added release notes for libraries. * Do not run integration tests with mono flag anymore. * Added warning when trying to use the Mono script engine. * Use MSBuild settings with 'dotnet publish' command.

devlead

LGTM 👍

bjorkstromm

LGTM!

patriksvensson requested review from gep13, agc93, devlead and bjorkstromm June 14, 2017 15:00

dnfclas added the cla-already-signed label Jun 14, 2017

patriksvensson changed the title ~~[WIP] Update to latest Roslyn~~ [WIP] GH1540: Update to latest Roslyn Jun 14, 2017

bjorkstromm previously requested changes Jun 14, 2017

View reviewed changes

patriksvensson requested a review from bjorkstromm July 23, 2017 23:06

bjorkstromm mentioned this pull request Jul 24, 2017

Several updates to build OmniSharp/omnisharp-roslyn#924

Merged

bjorkstromm reviewed Jul 24, 2017

View reviewed changes

bjorkstromm mentioned this pull request Jul 24, 2017

Log script compilation warnings and other diagnostics #1692

Closed

gep13 requested changes Jul 25, 2017

View reviewed changes

agc93 approved these changes Jul 26, 2017

View reviewed changes

bjorkstromm approved these changes Jul 26, 2017

View reviewed changes

gep13 approved these changes Jul 28, 2017

View reviewed changes

devlead requested changes Jul 29, 2017

View reviewed changes

patriksvensson and others added 5 commits July 29, 2017 18:41

Unified script engines.

4fbb422

* Updated projects to net462. * Cleaned up the project files. * Removed old versions of Roslyn. * Removed Mono scripting engine. * Added Roslyn script engine for full fx.

Package NuGet packages without nuspecs.

692bab6

* Removed unused build script variables. * Cleaned up the build script.

Use NET462 targeting pack when not running on Windows.

8ee4895

Improved debugging experience.

943d3d5

* Update Roslyn to 2.3.1. * Use standardized way of emitting debug symbols for debug build. * Unify script sessions.

devlead approved these changes Jul 29, 2017

View reviewed changes

bjorkstromm approved these changes Jul 29, 2017

View reviewed changes

devlead merged commit 1a2b10f into cake-build:develop Jul 29, 2017

This was referenced Jul 29, 2017

Upgrade to Roslyn 2.0 #1540

Closed

Please support C# 7 and Roslyn v2 #1512

Closed

This was referenced Aug 1, 2017

GH1710: GetSubDirectories alias #1711

Merged

Support For Interpolated String Literal Expressions #1691

Closed

GH1540: Update to latest Roslyn #1645

GH1540: Update to latest Roslyn #1645

Conversation

patriksvensson commented Jun 14, 2017 • edited Loading

bjorkstromm Jun 14, 2017

Choose a reason for hiding this comment

devlead commented Jun 26, 2017

devlead commented Jul 17, 2017

gep13 commented Jul 19, 2017

patriksvensson commented Jul 23, 2017

bjorkstromm commented Jul 24, 2017

patriksvensson commented Jul 24, 2017

bjorkstromm commented Jul 24, 2017

bjorkstromm commented Jul 24, 2017

filipw commented Jul 24, 2017

filipw commented Jul 24, 2017

bjorkstromm commented Jul 24, 2017

bjorkstromm commented Jul 24, 2017

gep13 commented Jul 24, 2017

filipw commented Jul 24, 2017

bjorkstromm Jul 24, 2017

Choose a reason for hiding this comment

patriksvensson Jul 24, 2017

Choose a reason for hiding this comment

bjorkstromm commented Jul 24, 2017

filipw commented Jul 24, 2017

patriksvensson commented Jul 24, 2017

gep13 left a comment

Choose a reason for hiding this comment

gep13 Jul 25, 2017

Choose a reason for hiding this comment

patriksvensson Jul 25, 2017

Choose a reason for hiding this comment

patriksvensson commented Jul 25, 2017

agc93 left a comment

Choose a reason for hiding this comment

bjorkstromm left a comment

Choose a reason for hiding this comment

devlead commented Jul 26, 2017

patriksvensson commented Jul 28, 2017

gep13 commented Jul 28, 2017

patriksvensson commented Jul 28, 2017

devlead commented Jul 28, 2017

devlead Jul 29, 2017

Choose a reason for hiding this comment

devlead left a comment

Choose a reason for hiding this comment

bjorkstromm commented Jul 29, 2017

devlead commented Jul 29, 2017

devlead left a comment

Choose a reason for hiding this comment

bjorkstromm left a comment

Choose a reason for hiding this comment

patriksvensson commented Jun 14, 2017 •

edited

Loading