MSBuildHelper: can't add a logger (/logger)

[0x53A]

I need to add the StructuredLogger to my build.

From the readme, I need to emit a cmdline like this:

msbuild solution.sln /t:Rebuild /v:diag /noconlog /logger:BinaryLogger,%localappdata%\MSBuildStructuredLogViewer\app-1.1.168\BinaryLogger.dll;1.binlog

Currently I hacked it like this:

    MSBuildHelper.MSBuildLoggers <- (sprintf "StructuredLogger,\"%s\";%s" ((getExtractedToolsDir()) @@ @"Microsoft.Build.Logging.StructuredLogger\lib\net46\StructuredLogger.dll") (logDir @@ "TimPrecast.buildlog") ) :: MSBuildHelper.MSBuildLoggers
    MSBuildHelper.MSBuildLoggers <- (sprintf "BinaryLogger,\"%s\";%s" ((getExtractedToolsDir()) @@ @"Microsoft.Build.Logging.StructuredLogger\lib\net46\StructuredLogger.dll") (logDir @@ "TimPrecast.binlog") ) :: MSBuildHelper.MSBuildLoggers

But it would obviously be preferrable if I didn't have to hack the global state.

There is FileLoggers with emits /fl and DistributedLoggers, which emits /dl, but there seems to be nothing that emits /logger.

This could be solved either by adding an explicit property Loggers, or by just adding a string list AdditionalArguments.

Thoughts?

[matthid]

As we have already discussed I'd like the msbuild module to be reworked :P.

A nice side effect would be that this particular feature would be easy to add. And we could have convenience high order functions to add a particular logger to the arguments...

[0x53A]

If I were to consider PRing this...

I'd add two properties, Loggers which maps to /logger and AdditionalArguments, which is a string list and just gets appended to the end.

Would you agree?

[matthid]

@0x53A Maybe/probably, I'd have to see in the PR the exact changes :)

[vbfox]

From our codebase (FAKE 4 for now):

    let setBinLog (path: string) (p: MSBuildParams): MSBuildParams =
        // The current version of FAKE doesn't support binlog, so we "add" it by exploiting the fact that Properties aren't escaped
        // "AdditionalProperties" is just a convenient one, we could use any property :)

        let props = p.Properties |> Map.ofList
        let additional = defaultArg (props |> Map.tryFind "AdditionalProperties") ""
        let hackedUpAdditional = sprintf "%s\" \"/bl:%s" additional path
        let hackedUpProps = props |> Map.add "AdditionalProperties" hackedUpAdditional |> Map.toList
        { p with Properties = hackedUpProps }

...

[matthid]

Relevant for #2096 (comment)