Skip to content
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.

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

FscHelper.compile fails with multiple references #1341

Closed
ErikSchierboom opened this issue Jul 30, 2016 · 21 comments
Closed

FscHelper.compile fails with multiple references #1341

ErikSchierboom opened this issue Jul 30, 2016 · 21 comments

Comments

@ErikSchierboom
Copy link
Contributor

Description

The FscHelper.compile function requires assembly references. Explicitly specifying the arguments using FscHelper.compileFiles fixes the problem.

This is the code I used within my FAKE script to dynamically compile a set of F# files:

FscHelper.compile
        [FscHelper.Out output;
         FscHelper.References [
            "tools/NUnit/lib/net45/nunit.framework.dll";
            "tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsecCS.dll";
            "tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsec.dll"]
         FscHelper.Target FscHelper.TargetType.Library]
        files

When this code is executed, the following error occurs:

startup (1,1)-(1,81) parse error Unable to find the file 'tools/NUnit/lib/net45/nunit.framework.dll -r:tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsecCS.dll -r:tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsec.dll' in any of
 /Library/Frameworks/Mono.framework/Versions/4.4.1/lib/mono/4.5
 /Users/erikschierboom/Programming/xfsharp
 /Users/erikschierboom/Programming/xfsharp/tools/FAKE/tools

At first I thought it had something to do with relative paths, but then I tried using FscHelper.compileFiles:

FscHelper.compileFiles files  
        ["--target:library";
         sprintf "--out:%s" output;
         "-r:tools/NUnit/lib/net45/nunit.framework.dll";
         "-r:tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsec.dll";
         "-r:tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsecCS.dll" ]

Funnily enough, this works! I then noted that the order of the parameters was different, so I changed my original version to re-order the parameters:

FscHelper.compile
    [FscHelper.Out output;
     FscHelper.Target FscHelper.TargetType.Library
     FscHelper.References [
         "tools/NUnit/lib/net45/nunit.framework.dll";
         "tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsecCS.dll";
         "tools/FParsec/lib/portable-net45+netcore45+wpa81+wp8/FParsec.dll"]
    ]
    files

Unfortunately, this also leads to the same error as before. If I remove all but one reference, things work again:

FscHelper.compile
        [FscHelper.Out output;
         FscHelper.References [
            "tools/NUnit/lib/net45/nunit.framework.dll"]
         FscHelper.Target FscHelper.TargetType.Library
         ]
        files

This time, I get a compile error that is due to the missing DLL references, but I do not get the parse error.

Repro steps

Please provide the steps required to reproduce the problem

  1. Step A

Checkout this repro.

  1. Step B

Run the build.cmd or build.sh command to see the build fail.

  1. Step C

Comment out the invalid FscHelper.compile code and uncomment the FscHelper.compileFiles code.

  1. Step D

Run the build.cmd or build.sh command to see the build succeed.

Expected behavior

The FscHelper.compile function should correctly handle multiple references.

Actual behavior

Compilation fails when using multiple references in the FscHelper.compile function.

Known workarounds

Use FscHelper.compileFiles.

Related information

  • Windows and Mac OS X
  • Latest (4.35.1)
  • .NET 4.5 and Mono 4.4.1
@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

I had a look into it

The problems are in https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/FscHelper.fs#L395.
This should probably better read

| References dllPaths -> dllPaths 
                         |> List.map (sargp "r") 
                         |> List.map (fun x -> sprintf "\"%s\"" x)
                         |> String.concat " "

as every single DLL reference needs to be wrapped in ".

The next problematic line is https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/FscHelper.fs#L499
This wraps the whole param (in this case a list of DLL-references) into ". Which in this particular case it shouldnt.

One easy way (alas a hacky way imo) would be to drop the first char and the last char of the generated string on the match case so.

| References dllPaths -> dllPaths 
                         |> List.map (sargp "r") 
                         |> List.map (fun x -> sprintf "\"%s\"" x)
                         |> String.concat " "
                         |> (fun x -> x.Substring(1, x.Length - 2))

It isnt exactly nice but it probably would do the job.

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

And as a side note: Instead of falling back to using compileFiles you can still use compile but instead of References use Reference (without the ending s) and wrap every single of them

 compile
    [
      Out output;
       Reference "Foo.dll"
       Reference "Bar.dll";
    ]
    files

@ErikSchierboom
Copy link
Contributor Author

Thanks for the tip!

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

This can be closed - my fix got merged already

@ErikSchierboom
Copy link
Contributor Author

Thanks!

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

shoot! - we need to reopen. There is still a bug. Seems like inbetween all prams we need a semicolon.

@forki
Copy link
Member

forki commented Aug 9, 2016

just send a PR. will release hotfix asap.

@forki forki reopened this Aug 9, 2016
@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

Hi,

I created a new PR. Can you test it manually before doing a new release?
I am not really able for now to run a test in my environment. sorry.

Something like

["A.fs"; "B.fs"]
|> Compile 
    [   
        Out ("SomeApp.exe") 
        Target Exe
        Platform AnyCpu
        References ["YourDLL.dll"; "SomeOtherDLL.dll"]
        Debug true 
    ]

The "output" should be something like

FSC with args:[|"--out:/SomeApp.exe"; "--target:exe"; "--platform:anycpu";
  "-r:YourDLL.dll"; "-r:SomeOtherDLL.dll"; 
  "-g"; "A.fs"; "B.fs"|]

Obviously with real files and DLLs

@forki
Copy link
Member

forki commented Aug 9, 2016

since it's broken right now it's much easier for me to just merge and release. You can test it in about 10min

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

OK

@forki
Copy link
Member

forki commented Aug 9, 2016

and thanks for your help!

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

still doesnt work!
The only difference that remains is that when using single Reference values (which works) instead of References the "output" shown has a linefeed in between the references.

BUT I dont believe that this is the reason.

@forki
Copy link
Member

forki commented Aug 9, 2016

so what do we do? we can just try it?

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

I have copied the output that is printed when i use compile and References verbatim into compileFiles which is used internally by compile. And it works! WTF?

yeah the only thing we can try is to insert additional line feeds \n and hope for the best. If that doesnt work I am out of ideas. plus: would we need to carter for platform or does \n work?

@forki
Copy link
Member

forki commented Aug 9, 2016

can insert Environment.NewLine?

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

will try

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

sigh still the same. I will have to have a very close look what compileFiles does. As I dont see any differences (apart from some whitespaces) now. Alas this will have to wait for tomorrow

@forki
Copy link
Member

forki commented Aug 9, 2016

should I revert to a specific commit?

@robkuz
Copy link
Contributor

robkuz commented Aug 9, 2016

well basically back to 4.36 I'd say. Or you leave it on 4.37 as in 4.36 the same bug is present. so this should in theory be closer to a solution.

@forki
Copy link
Member

forki commented Aug 9, 2016

Ok so I will just wait and hope you will find a way. Thanks for taking care
of this. Really appreciated

On Aug 9, 2016 15:36, "robertj" notifications@github.com wrote:

well basically back to 4.36 I'd say. Or you leave it on 4.37 as in 4.36
the same bug is present. so this should in theory be closer to a solution.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#1341 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADgNMjkya0w2cPir2aXzNMPuP6d-x26ks5qeIJtgaJpZM4JYwLf
.

@matthid
Copy link
Member

matthid commented May 7, 2017

So is this still a problem?

@matthid matthid closed this as completed May 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants