Updating Roslyn packages to 2.0.0 #1367
Conversation
|
Because of the package updates, we need to validate that we won't have issues with NGEN. |
| @@ -8,8 +8,7 @@ public static Task<HttpResponseMessage> Run(HttpRequestMessage req, TraceWriter | |||
| log.Info(string.Format("C# HTTP trigger function processed a request. {0}", req.RequestUri)); | |||
|
|
|||
| HttpResponseMessage res = null; | |||
| string name; | |||
| if (queryParams.TryGetValue("name", out name)) | |||
| if (queryParams.TryGetValue("name", out string name)) | |||
There was a problem hiding this comment.
Small change to use C# 7 syntax here.
There was a problem hiding this comment.
A couple of ngen comments -- I'm fairly confident that we can get away with the redirects (and will need to soon), but would be good to think through again.
Also, side note, @tohling was hopeful that moving these forward would eliminate the ngen warnings we were seeing (dotnet/roslyn#16261), but I did a really quick trial using them last week and I still saw the warnings. We may need to revive that issue with the Roslyn team to see if we can figure it out.
test/WebJobs.Script.Tests/app.config
Outdated
| </dependentAssembly> | ||
| <dependentAssembly> | ||
| <assemblyIdentity name="Autofac" publicKeyToken="17863af14b0044da" culture="neutral" /> | ||
| <bindingRedirect oldVersion="0.0.0.0-4.2.1.0" newVersion="4.2.1.0" /> | ||
| </dependentAssembly> | ||
| <dependentAssembly> | ||
| <assemblyIdentity name="System.IO.Compression" publicKeyToken="b77a5c561934e089" culture="neutral" /> | ||
| <bindingRedirect oldVersion="0.0.0.0-4.1.2.0" newVersion="4.1.2.0" /> |
There was a problem hiding this comment.
I think this will introduce the ngen issue that I was seeing... This is one of the ones that they suggest directing back to 4.0.0.0: https://github.com/dotnet/corefx/issues/15693
There was a problem hiding this comment.
I'll push this back to 4.0.
Are you still in a position where you can run an NGEN test?
There was a problem hiding this comment.
When are you planning to push his fix out?
FYI, there is a new OS image scheduled for the current sprint. In the context of ngen, we have to make sure the Ngen'ed bits will work with both current and next OS versions. The new OS image will be available for testing soon. Current ETA is on 4/6.
Brett - The way this will affect you is that we will need to redeploy the stamp you are testing on with the new OS image and make sure that the Ngen'ed bits are working. I will keep you posted as soon as we get the image.
| @@ -73,6 +73,34 @@ | |||
| <assemblyIdentity name="FSharp.Core" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | |||
| <bindingRedirect oldVersion="0.0.0.0-4.4.0.0" newVersion="4.4.0.0" /> | |||
| </dependentAssembly> | |||
| <dependentAssembly> | |||
| <assemblyIdentity name="System.Xml.ReaderWriter" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> | |||
| <bindingRedirect oldVersion="0.0.0.0-4.1.0.0" newVersion="4.1.0.0" /> | |||
There was a problem hiding this comment.
You should be able to remove this redirect completely. This is a facade that redirects to the framework already. Having this confuses the ngen'ing.
There was a problem hiding this comment.
Yeah, the current state with binding redirects is just a mess.
fabiocav
force-pushed
the
roslynupd
branch
3 times, most recently
from
748e861 to
59ced6e
Compare
|
Ran end-to-end NGEN tests. Clean NGEN runs. Merging. |
resolves #999
Adds support for C# 7