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

fsc compilation fix #601

Merged
merged 1 commit into from
Dec 3, 2014
Merged

fsc compilation fix #601

merged 1 commit into from
Dec 3, 2014

Conversation

sangkilc
Copy link
Contributor

@sangkilc sangkilc commented Dec 2, 2014

Since Fsc always skips the first argument, it is important to use a dummy argument when calling a Compile function. Otherwise, following compiler options can be ignored. For example, the current version of FAKE always ignores Output parameter, because "-o" is the first argument that appears in optsArr (thus, it is ignored along with the second argument that specifies the output file name). For your reference, Fsc compiler test code also includes "fsc.exe" as the first argument when calling the Compile function.

// But if it's not, then figure out what it should be.
else
let outExt =
if Seq.exists (fun e -> e = "-a" || e = "--target:library") opts then ".dll"
else ".exe"
"-o" :: FileHelper.changeExt outExt (List.head srcFiles) :: opts @ srcFiles
"fsc.exe" :: "-o" :: FileHelper.changeExt outExt (List.head srcFiles) :: opts @ srcFiles
Copy link
Member

Choose a reason for hiding this comment

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

Can we put it without duplication on the line where the Compile get called?

@sangkilc
Copy link
Contributor Author

sangkilc commented Dec 2, 2014

I just moved the cons to the top.

@@ -50,6 +50,8 @@ let fscList (srcFiles : string list) (opts : string list) : int =
let scs = SimpleSourceCodeServices()

let optsArr =
// Always prepend "fsc.exe" since fsc compiler skips the first argument
"fsc.exe" ::
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to bother, I forgot to mention the other reason why it should be moved down.
optsArr will be traced out, we don't need fsc.exe in that trace message.

@sangkilc
Copy link
Contributor Author

sangkilc commented Dec 2, 2014

Okay that makes sense to me.

@forki
Copy link
Member

forki commented Dec 3, 2014

thanks.

forki added a commit that referenced this pull request Dec 3, 2014
@forki forki merged commit 935f878 into fsprojects:master Dec 3, 2014
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

Successfully merging this pull request may close these issues.

3 participants