phmonte / Buildalyzer

A utility to perform design-time builds of .NET projects without having to think too hard about it.
MIT License
589 stars 92 forks source link

Buildalyzer hangs if build does not start #271

Closed slang25 closed 1 month ago

slang25 commented 1 month ago

@martincostello found that adding global properties with ; in would result in Buildalyzer hanging.

I've done some investigating and have created a minimal repro here: https://github.com/phmonte/Buildalyzer/compare/main...slang25:Buildalyzer:msbuild-hang-repro

If you run that test you will see that it just hangs. There are really two issues here, the first is minor, which is that we should escape the property values as outlined here to work around the fact that msbuild doesn't like ; in property values.

The second is a bit more fundamental, if the build doesn't start for whatever reason, Buildalyzer should not hang. I've been thinking about this and looking into the best way of supporting this, and there's not an obvious fix due to the way AnonymousPipeLoggerServer works, but the simple workaround I've found is to dispose of the underlying stream when:

  1. We've received no build events
  2. The process has exited with a non-zero return code

With this in place we should still get an overall result from Buildalyzer, but with an OverallSuccess of false as expected.

I'll create two PRs, one fixing the value escaping, the other with a (not ideal) fix for the general hang scenario.