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

Add MSBuild binary logger support #1649

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

kentcb
Copy link
Contributor

@kentcb kentcb commented Sep 1, 2017

Fixes #1631 by adding support for binary loggers to MSBuild (both the .NET Core and regular flavours).

Note that the iOSBuild function does not currently support loggers in its params. However, it is merely a facade onto the build function, so I don't really understand why it exists. Instead of attempting to add support for loggers and other missing bits into iOSBuildParams, I just changed my build script from using iOSBuild to build. For anyone wondering, the basic invocation looks like this:

    build (fun defaults ->
        {
            defaults with
                Verbosity = Some(Detailed)
                Targets = ["Rebuild"]
                Properties =
                    [
                        "Optimize", "True"
                        "Configuration", iosConfiguration
                        "Platform", iosPlatform
                        "BuildIpa", "true"
                    ]
                BinaryLoggers = Some
                    [
                        logFile
                    ]
                NoConsoleLogger = true
        })
        iosSolution

Please excuse whitespace changes - I have an addin that strips superfluous whitespace. If this is a problem, I can add the whitespace back in with a separate commit - let me know.

@kentcb
Copy link
Contributor Author

kentcb commented Sep 1, 2017

Build failure appears unrelated to my changes...?


1) Error : Fake.Core.IntegrationTests.SimpleHelloWorldTests.reference fake core targets
System.Exception : FAKE Process exited with 1: 
   at Fake.Core.IntegrationTests.TestHelpers.directFakeInPath@38.Invoke(String message) in C:\projects\fake-6w516\src\test\Fake.Core.IntegrationTests\TestHelpers.fs:line 38
   at Fake.Core.IntegrationTests.TestHelpers.directFakeInPath(String command, String scenarioPath, String target) in C:\projects\fake-6w516\src\test\Fake.Core.IntegrationTests\TestHelpers.fs:line 38
   at Fake.Core.IntegrationTests.TestHelpers.fakeRun(String scriptName, String scenario) in C:\projects\fake-6w516\src\test\Fake.Core.IntegrationTests\TestHelpers.fs:line 54

@kentcb
Copy link
Contributor Author

kentcb commented Sep 4, 2017

@forki any idea what's going on with the broken build? Seems like a number of PRs are failing due to the above.

@matthid
Copy link
Member

matthid commented Sep 4, 2017

Yes I tried to get it up again but couldn't make it (breaking changes in fcs and problems with paket)... Once it works again there will be another release, then I'll upgrade to netcoreapp20/netstandard20 and finally look at the new PRs. That will take a couple of days at least.

@matthid
Copy link
Member

matthid commented Sep 4, 2017

Ok master is "green" for the moment (there still is a transient error from time to time). Please try to rebase/merge.

@kentcb
Copy link
Contributor Author

kentcb commented Sep 4, 2017

Thanks @matthid - rebased and all green.

Copy link
Member

@matthid matthid left a comment

Choose a reason for hiding this comment

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

Looks good thanks

@matthid matthid merged commit 2bfdbfb into fsprojects:master Sep 18, 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

Successfully merging this pull request may close these issues.

2 participants