Skip to content

Use C# code instead of manually modified DLL. #4514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brettfo merged 5 commits into from
Mar 15, 2018
Merged

Use C# code instead of manually modified DLL. #4514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43cf495
Use C# code instead of manually modified DLL.
0x53A Mar 14, 2018
4fcf640
set /langversion:latest
0x53A Mar 14, 2018
314cf5c
pre-fill CSC_PIPE with Microsoft.Net.Compilers
0x53A Mar 14, 2018
8d51c8b
Merge branch 'master' of https://github.com/Microsoft/visualfsharp in…
0x53A Mar 15, 2018
7582e4c
upgrade Microsoft.Net.Compilers 2.4 => 2.7
0x53A Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ set HOSTED_COMPILER=1

if "%TEST_NET40_FSHARPQA_SUITE%" == "1" (

set CSC_PIPE=%~dp0packages\Microsoft.Net.Compilers.2.7.0\tools\csc.exe
set FSC=!FSCBINPATH!\fsc.exe
set FSCOREDLLPATH=!FSCBinPath!\FSharp.Core.dll
set PATH=!FSCBINPATH!;!PATH!
Expand Down
2 changes: 1 addition & 1 deletion tests/fsharp/packages.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.Net.Compilers" version="2.4.0" />
<package id="Microsoft.Net.Compilers" version="2.7.0" />
<package id="NUnit3TestAdapter" version="3.7.0" targetFramework="net45" />
<package id="runtime.win-x64.Microsoft.NETCore.ILDAsm" version="2.0.3" />
<package id="runtime.win-x86.Microsoft.NETCore.ILDAsm" version="2.0.3" />
Expand Down
2 changes: 1 addition & 1 deletion tests/fsharp/test-framework.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ let config configurationName envVars =
let fsi_flags = "-r:System.Core.dll --nowarn:20 --define:INTERACTIVE --maxerrors:1 --abortonerror"
let Is64BitOperatingSystem = WindowsPlatform.Is64BitOperatingSystem envVars
let architectureMoniker = if Is64BitOperatingSystem then "x64" else "x86"
let CSC = requireFile (packagesDir ++ "Microsoft.Net.Compilers.2.4.0" ++ "tools" ++ "csc.exe")
let CSC = requireFile (packagesDir ++ "Microsoft.Net.Compilers.2.7.0" ++ "tools" ++ "csc.exe")
let ILDASM = requireFile (packagesDir ++ ("runtime.win-" + architectureMoniker + ".Microsoft.NETCore.ILDAsm.2.0.3") ++ "runtimes" ++ ("win-" + architectureMoniker) ++ "native" ++ "ildasm.exe")
let coreclrdll = requireFile (packagesDir ++ ("runtime.win-" + architectureMoniker + ".Microsoft.NETCore.Runtime.CoreCLR.2.0.3") ++ "runtimes" ++ ("win-" + architectureMoniker) ++ "native" ++ "coreclr.dll")
let PEVERIFY = requireFile (SCRIPT_ROOT ++ ".." ++ "fsharpqa" ++ "testenv" ++ "src" ++ "PEVerify" ++ "bin" ++ configurationName ++ "net46" ++ "PEVerify.exe")
Expand Down
6 changes: 2 additions & 4 deletions tests/fsharpqa/Source/Import/AccessibilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ public class Accessibility
private int Private { get; set; }
protected int Protected { get; set; }
internal int Internal { get; set; }
// Note: accessibility was modified using dnspy
public int FamOrAssembly { get; set; }
// Note: accessibility was modified using dnspy
public int FamAndAssembly { get; set; }
protected internal int FamOrAssembly { get; set; }
private protected int FamAndAssembly { get; set; }
}

Binary file removed tests/fsharpqa/Source/Import/AccessibilityTests.dll
View file
Open in desktop
Binary file not shown.
10 changes: 5 additions & 5 deletions tests/fsharpqa/Source/Import/env.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ NOMONO SOURCE=reference2.fsx SCFLAGS="--nologo -r:reference1.dll" PRECMD="\$FSC_
###
### F# can consume FamOrAssembly and FamAndAssembly in combination with IVT
###
SOURCE=FamAndAssembly.fs SCFLAGS="-a -r:AccessibilityTests.dll" # FamAndAssembly.fs
SOURCE=FamAndAssembly.fs SCFLAGS="-a -r:AccessibilityTests.dll" PRECMD="\$CSC_PIPE /debug /target:library /langversion:latest AccessibilityTests.cs" # FamAndAssembly.fs
### See issue https://github.com/Microsoft/visualfsharp/issues/2496
### 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
Copy link

@sliepie sliepie Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@forki forki Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes "latest" is not a good idea

Copy link
Member

@brettfo brettfo Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@0x53A 0x53A Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member

@brettfo brettfo Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@0x53A 0x53A Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be enough if I set CSC_PIPE from externally, right?

https://github.com/Microsoft/visualfsharp/blob/2d2104b1fce6ed0bae26695f28fb9913c9ff9bf5/tests/fsharpqa/Source/run.pl#L140-L143

Here it seems to read the env-var, and only defaults it to csc if it isn't already set.

Copy link
Member

@brettfo brettfo Mar 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, simpler is better.

SOURCE=FamAndAssembly_NoIVT.fs SCFLAGS="-a -r:AccessibilityTests.dll" PRECMD="\$CSC_PIPE /debug /target:library /langversion:latest AccessibilityTests.cs" # FamAndAssembly_NoIVT.fs
### SOURCE=FamOrAssembly_NoIVT.fs SCFLAGS="-a -r:AccessibilityTests.dll" PRECMD="\$CSC_PIPE /debug /target:library /langversion:latest AccessibilityTests.cs" # FamOrAssembly_NoIVT.fs

###
### Iterate over BCL collections
Expand All @@ -100,4 +100,4 @@ NOMONO SOURCE=reference2.fsx SCFLAGS="--nologo -r:reference1.dll" PRECMD="\$FSC_


SOURCE=ReferenceExe01.fsx PRECMD="\$FSC_PIPE ReferenceExe.fs" # ReferenceExe01.fsx
SOURCE=LineDirectiveFromCSharp.fs PRECMD="\$CSC_PIPE /t:library LineDirectiveLib.cs" SCFLAGS="-r:LineDirectiveLib.dll" # LineDirectiveFromCSharp.fs
SOURCE=LineDirectiveFromCSharp.fs PRECMD="\$CSC_PIPE /t:library LineDirectiveLib.cs" SCFLAGS="-r:LineDirectiveLib.dll" # LineDirectiveFromCSharp.fs