Use C# code instead of manually modified DLL. #4514
Conversation
|
| ### SOURCE=FamOrAssembly.fs SCFLAGS="-a -r:AccessibilityTests.dll" # FamOrAssembly.fs | ||
| SOURCE=FamAndAssembly_NoIVT.fs SCFLAGS="-a -r:AccessibilityTests.dll" # FamAndAssembly_NoIVT.fs | ||
| ### SOURCE=FamOrAssembly_NoIVT.fs SCFLAGS="-a -r:AccessibilityTests.dll" # FamOrAssembly_NoIVT.fs | ||
| ### SOURCE=FamOrAssembly.fs SCFLAGS="-a -r:AccessibilityTests.dll" PRECMD="\$CSC_PIPE /debug /target:library /langversion:latest AccessibilityTests.cs" # FamOrAssembly.fs |
There was a problem hiding this comment.
Can we not specify 7.2 explicit? Depending of the version of VS/MSBuild, latest could mean a version lower than 7.2 e.g. C# version 5 or 6.
There was a problem hiding this comment.
yes "latest" is not a good idea
There was a problem hiding this comment.
Speaking with @KevinRansom we're OK with latest so long as CSC_PIPE is updated to use csc.exe from the NuGet dependency I added in #4023 (with the caveat that the C# package might need to be upgraded; I don't know when 7.2 features were added.)
Currently I think run.pl grabs a version of csc.exe from the PATH instead of the explicit package version.
There was a problem hiding this comment.
@brettfo Should I just hardcode it in run.pl (then it would need to be adapted each time you update), or can it be passed down from somewhere?
There was a problem hiding this comment.
Ideally something passed in, even if that passed in value is hard-coded in build.cmd.
Looking through the code you could add a hard-coded argument in build.cmd and then modify runall.pl to handle the new argument. I didn't immediately find a way to pass through a value to run.pl, but it appears to prefer an environment variable, so you could probably just do $ENV{CSC_PIPE} = $passedInPathToCsc; in runall.pl.
There was a problem hiding this comment.
It should be enough if I set CSC_PIPE from externally, right?
Here it seems to read the env-var, and only defaults it to csc if it isn't already set.
|
Good news is that the environment variable worked. Bad news is that either the C# syntax is wrong, the compiler is too old, or both. I don't know enough about the C# syntax to comment on that, but version 2.7.0 of Microsoft.Net.Compilers should be new enough for the added syntax. |
brettfo
left a comment
brettfo
left a comment
There was a problem hiding this comment.
I like the changes. @KevinRansom do you have any additional feedback?
4 participants
Now that C# natively supports these access modifiers, we don't need to rely on a checked-in binary anymore.