[WIP-FEATURE] MSBuild server node

[MichalPavlik]

Fixes #7372

Context

MSBuild server is a new node that accepts build request from client. This approach avoids spawning a new MSBuild process for every build from CLI tools like .NET SDK.

Changes Made

This PR implements a new node capable to accept and validate pipe connection, receive and invoke build commands, send console messages to client and inform about build result.

Testing

Tests will be added.

Notes

This is only draft as some areas need more care:

• Intelligent console messages flushing - based on buffer size AND elapsed time since last flush
• Console messages colorization - we want to use ANSI escape codes for colorization. Right now it's possible to pass -consoleLoggerParameters:ForceConsoleColor parameter from client, but we have to find a better way.
• Unit tests

Some aspects will be implemented in different PR:

1. Configurable timeout (nice to have)
2. Support for build cancellation
3. Non-interactive console checking - we don't want to emit ASNI escape codes when the stream is redirected

[ladipro]

Leaving a few comments inline.

[ladipro]

nit: Better make it a class because the struct will be boxed on the way out of GetHandshake anyway.

[ladipro]

If I'm reading the sources correctly, the only place where handshake needs to be accessed in a generic way is NodeEndpointOutOfProcBase.GetHandshake. This has exactly one caller in PacketPumpProc and only one method is called on the handshake: RetrieveHandshakeComponents.

So overall it looks like this interface may be an overkill and we could instead have something like RetrieveHandshakeComponents (suggestion: name it GetHandshakeData?) directly on NodeEndpointOutOfProcBase.

[ladipro]

nit: Some of this code looks copied from the original Handshake. Do you think it would be worth deduping? By introducing a base class or a helper method maybe.

[Forgind]

Can you explain why we need a new handshake at all? The point of the original handshake is that both sides need to come up with the same thing. I don't like the idea of adding a new thing here without good justification.

[MichalPavlik]

I had same question. Handshake from prototype was a little bit different, but I had to align it with current codebase, so it looks almost same now. The main difference is that server doesn't need session ID. We can remove duplicates by (for example) inheritance - if it's OK to change the original handshake from value type to reference type.

cc: @rokonec

[danmoseley]

I didn't review the code but it's of course critical that the CLI-MSBuild pipe is tested to ensure it's secured in the same way as the MSBuild-MSBUild pipes are: the user at both ends must be the same user and (on Windows) elevation level.

In .NET Core there's API to do this: while MSBuild builds against .NET Framework it has to continue to do it manually.

[MichalPavlik]

Hello @danmoseley, client-server communication uses same IPC infrastructure with user and elevation checking as the current nodes. It's implemented in base class and we didn't change it.

[rokonec]

This race condition will be most probably handled by client. Please change this command to describe our decision about how will client handle it.

[AR-May]

There would be mutex "Global\server-launch-{_pipeName}" for starting server process in a client. This does not guarantee that some other client, if anybody writes it, would not introduce a race condition, so the fast quit from server process in this case is still needed.

[Forgind]

If this is no longer a WIP, can you remove the [WIP-FEATURE] in the title?

Looks like the test failures are from an overload of GetHandshakeOptions not being found.

[MichalPavlik]

@Forgind, we marked it as WIP as we will merge the code to the feature branch with the client implementation. It will make the work more efficient and we will continue with changes.